Re: [PATCH] 9pfs: Fix some return statements in the synth backend
Greg Kurz writes: > The qemu_v9fs_synth_mkdir() and qemu_v9fs_synth_add_file() functions > currently return a positive errno value on failure. This causes > checkpatch.pl to spit several errors like the one below: > > ERROR: return of an errno should typically be -ve (return -EAGAIN) > #79: FILE: hw/9pfs/9p-synth.c:79: > +return EAGAIN; > > Simply change the sign. This has no consequence since callers > assert() the returned value to be equal to 0. Out of curiosity: why is assert() appropriate? > While here also get rid of the uneeded ret variables as suggested > by return_directly.cocci. > > Reported-by: Markus Armbruster > Signed-off-by: Greg Kurz Signed-off-by: Markus Armbruster
[qemu-web PATCH v2] Add a blog post about zoned storage emulation
Signed-off-by: Sam Li --- _posts/2022-11-17-zoned-emulation.md | 69 1 file changed, 69 insertions(+) create mode 100644 _posts/2022-11-17-zoned-emulation.md diff --git a/_posts/2022-11-17-zoned-emulation.md b/_posts/2022-11-17-zoned-emulation.md new file mode 100644 index 000..1e16e99 --- /dev/null +++ b/_posts/2022-11-17-zoned-emulation.md @@ -0,0 +1,69 @@ +--- +layout: post +title: "Introduction to Zoned Storage Emulation" +date: 2022-11-17 +author: Sam Li +categories: [storage, gsoc, outreachy, internships] +--- + +This summer I worked on adding Zoned Block Device (ZBD) support to virtio-blk as part of the [Outreachy](https://www.outreachy.org/) internship program. QEMU hasn't directly supported ZBDs before so this article explains how they work and why QEMU needed to be extended. + +## Zoned block devices + +Zoned block devices (ZBDs) are divided into regions called zones that can only be written sequentially. By only allowing sequential writes, SSD write amplification can be reduced by eliminating the need for a [Flash Translation Layer](https://en.wikipedia.org/wiki/Flash_translation_layer), and potentially lead to higher throughput and increased capacity. Providing a new storage software stack, zoned storage concepts are standardized as [ZBC (SCSI standard), ZAC (ATA standard)](https://zonedstorage.io/docs/introduction/smr#governing-standards), and [ZNS (NVMe)](https://zonedstorage.io/docs/introduction/zns). Meanwhile, the virtio protocol for block devices(virtio-blk) should also be aware of ZBDs instead of taking them as regular block devices. It should be able to pass such devices through to the guest. An overview of necessary work is as follows: + +1. Virtio protocol: [extend virtio-blk protocol with main zoned storage concept](https://lwn.net/Articles/914377/), Dmitry Fomichev +2. Linux: [implement the virtio specification extensions](https://www.spinics.net/lists/linux-block/msg91944.html), Dmitry Fomichev +3. QEMU: [add zoned storage APIs to the block layer](https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg05195.html), Sam Li +4. QEMU: implement zoned storage support in virtio-blk emulation, Sam Li + +Once the QEMU and Linux patches have been merged it will be possible to expose a virtio-blk ZBD to the guest like this: + +```sh +-blockdev node-name=drive0,driver=zoned_host_device,filename=/path/to/zbd,cache.direct=on \ +-device virtio-blk-pci,drive=drive0 \ +``` + +And then we can perform zoned block commands on that device in the guest os. + +```sh +# blkzone report /dev/vda + start: 0x0, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 0(nw) [type: 1(CONVENTIONAL)] + start: 0x2, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 0(nw) [type: 1(CONVENTIONAL)] + start: 0x4, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 0(nw) [type: 1(CONVENTIONAL)] + start: 0x6, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 0(nw) [type: 1(CONVENTIONAL)] + start: 0x8, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 0(nw) [type: 1(CONVENTIONAL)] + start: 0xa, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 0(nw) [type: 1(CONVENTIONAL)] + start: 0xc, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 0(nw) [type: 1(CONVENTIONAL)] + start: 0xe, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 0(nw) [type: 1(CONVENTIONAL)] + start: 0x00010, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 1(em) [type: 2(SEQ_WRITE_REQUIRED)] + start: 0x00012, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 1(em) [type: 2(SEQ_WRITE_REQUIRED)] + start: 0x00014, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 1(em) [type: 2(SEQ_WRITE_REQUIRED)] + start: 0x00016, len 0x02, cap 0x02, wptr 0x00 reset:0 non-seq:0, zcond: 1(em) [type: 2(SEQ_WRITE_REQUIRED)] +``` + +## Zoned emulation + +Currently, QEMU can support zoned devices by virtio-scsi or PCI device passthrough. It needs to specify the device type it is talking to. Whereas storage controller emulation uses block layer APIs instead of directly accessing disk images. Extending virtio-blk emulation avoids code duplication and simplify the support by hiding the device types under a unified zoned storage interface, simplifying VM deployment for different types of zoned devices. Virtio-blk can also be implemented in hardware. If those devices wish to follow the zoned storage model then the virtio-blk specification needs to natively support zoned storage. With such support, individual NVMe namespaces or anything that is a zoned Linux block device can be exposed to the guest without passing through a full device. + +For zoned storage emulation, zoned storage APIs support three zoned
Re: [PATCH] hw/display/next-fb: Fix comment typo
On 25/11/22 17:08, Evgeny Ermakov wrote: Signed-off-by: Evgeny Ermakov --- hw/display/next-fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] tests/qtest/migration-test: Fix unlink error and memory leaks
On Fri, Nov 25, 2022 at 4:33 PM Thomas Huth wrote: > > When running the migration test compiled with Clang from Fedora 37 > and sanitizers enabled, there is an error complaining about unlink(): > > ../tests/qtest/migration-test.c:1072:12: runtime error: null pointer > passed as argument 1, which is declared to never be null > /usr/include/unistd.h:858:48: note: nonnull attribute specified here > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > ../tests/qtest/migration-test.c:1072:12 in > (test program exited with status code 1) > TAP parsing error: Too few tests run (expected 33, got 20) > > The data->clientcert and data->clientkey pointers can indeed be unset > in some tests, so we have to check them before calling unlink() with > those. > > While we're at it, I also noticed that the code is only freeing > some but not all of the allocated strings in this function, and > indeed, valgrind is also complaining about memory leaks here. > So let's call g_free() on all allocated strings to avoid leaking > memory here. > > Signed-off-by: Thomas Huth > --- > tests/qtest/migration-test.c | 20 > 1 file changed, 16 insertions(+), 4 deletions(-) > Tested-by: Bin Meng
Re: [Virtio-fs] [PATCH] virtiofsd: Add `sigreturn` to the seccomp whitelist
Am 25.11.22 um 17:32 schrieb German Maglione:
On Fri, Nov 25, 2022 at 3:40 PM Marc Hartmayer wrote:
The virtiofsd currently crashes on s390x. This is because of a
`sigreturn` system call. See audit log below:
type=SECCOMP msg=audit(1669382477.611:459): auid=4294967295 uid=0 gid=0 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 pid=6649 comm="virtiofsd" exe="/usr/libexec/virtiofsd" sig=31
arch=8016 syscall=119 compat=0 ip=0x3fff15f748a code=0x8000AUID="unset" UID="root"
GID="root" ARCH=s390x SYSCALL=sigreturn
Signed-off-by: Marc Hartmayer
---
tools/virtiofsd/passthrough_seccomp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virtiofsd/passthrough_seccomp.c
b/tools/virtiofsd/passthrough_seccomp.c
index 888295c073de..0033dab4939e 100644
--- a/tools/virtiofsd/passthrough_seccomp.c
+++ b/tools/virtiofsd/passthrough_seccomp.c
@@ -110,6 +110,7 @@ static const int syscall_allowlist[] = {
#endif
SCMP_SYS(set_robust_list),
SCMP_SYS(setxattr),
+SCMP_SYS(sigreturn),
SCMP_SYS(symlinkat),
SCMP_SYS(syncfs),
SCMP_SYS(time), /* Rarely needed, except on static builds */
--
2.34.1
___
Virtio-fs mailing list
virtio...@redhat.com
https://listman.redhat.com/mailman/listinfo/virtio-fs
Reviewed-by: German Maglione
Should we add this also in the rust version?, I see we don't have it
enabled either.
this is probably a good idea.
Re: [PATCH v2] riscv: Add RISCVCPUConfig.satp_mode to set sv48, sv57, etc.
On Fri, Nov 25, 2022 at 7:01 PM Alexandre Ghiti wrote:
>
> RISC-V specifies multiple sizes for addressable memory and Linux probes for
> the machine's support at startup via the satp CSR register (done in
> csr.c:validate_vm).
>
> As per the specification, sv64 must support sv57, which in turn must
> support sv48...etc. So we can restrict machine support by simply setting the
> "highest" supported mode in the satp_mode property. And the bare mode is
> always supported.
>
> You can set this new property as follows:
> -cpu rv64,satp-mode=sv48 # Linux will boot using sv48 scheme
> -cpu rv64,satp-mode=sv39 # Linux will boot using sv39 scheme
>
> In addition, we now correctly set the device-tree entry 'mmu-type' using
> this new satp_mode property.
>
> Reviewed-by: Atish Patra
> Co-Developed-by: Ludovic Henry
> Signed-off-by: Ludovic Henry
> Signed-off-by: Alexandre Ghiti
> ---
>
> v2:
> - Use error_setg + return as suggested by Alistair
> - Add RB from Atish
> - Fixed checkpatch issues missed in v1
> - Replaced Ludovic email address with the rivos one
>
> hw/riscv/virt.c| 15 ++-
> target/riscv/cpu.c | 45 +
> target/riscv/cpu.h | 3 +++
> target/riscv/csr.c | 8 ++--
> 4 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index a5bc7353b4..77484b5cae 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -228,7 +228,7 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int
> socket,
> int cpu;
> uint32_t cpu_phandle;
> MachineState *mc = MACHINE(s);
> -char *name, *cpu_name, *core_name, *intc_name;
> +char *name, *cpu_name, *core_name, *intc_name, *sv_name;
>
> for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) {
> cpu_phandle = (*phandle)++;
> @@ -236,14 +236,11 @@ static void create_fdt_socket_cpus(RISCVVirtState *s,
> int socket,
> cpu_name = g_strdup_printf("/cpus/cpu@%d",
> s->soc[socket].hartid_base + cpu);
> qemu_fdt_add_subnode(mc->fdt, cpu_name);
> -if (riscv_feature(>soc[socket].harts[cpu].env,
> - RISCV_FEATURE_MMU)) {
> -qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
> -(is_32_bit) ? "riscv,sv32" :
> "riscv,sv48");
> -} else {
> -qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
> -"riscv,none");
> -}
> +
> +sv_name = g_strdup_printf("riscv,%s",
> +
> s->soc[socket].harts[cpu].cfg.satp_mode_str);
> +qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type", sv_name);
> +
Missing sv_name free
> name = riscv_isa_string(>soc[socket].harts[cpu]);
> qemu_fdt_setprop_string(mc->fdt, cpu_name, "riscv,isa", name);
> g_free(name);
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d14e95c9dc..c86dc5058d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -907,6 +907,48 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
> }
> #endif
>
> +/*
> + * Either a cpu sets its supported satp_mode in XXX_cpu_init
> + * or the user sets this value using satp_mode property.
> + */
> +bool rv32 = riscv_cpu_mxl(>env) == MXL_RV32;
> +if (cpu->cfg.satp_mode_str) {
> +if (!g_strcmp0(cpu->cfg.satp_mode_str, "none"))
> +cpu->cfg.satp_mode = VM_1_10_MBARE;
> +else if (!g_strcmp0(cpu->cfg.satp_mode_str, "sv32") && rv32)
> +cpu->cfg.satp_mode = VM_1_10_SV32;
> +else if (!g_strcmp0(cpu->cfg.satp_mode_str, "sv39") && !rv32)
> +cpu->cfg.satp_mode = VM_1_10_SV39;
> +else if (!g_strcmp0(cpu->cfg.satp_mode_str, "sv48") && !rv32)
> +cpu->cfg.satp_mode = VM_1_10_SV48;
> +else if (!g_strcmp0(cpu->cfg.satp_mode_str, "sv57") && !rv32)
> +cpu->cfg.satp_mode = VM_1_10_SV57;
> +else if (!g_strcmp0(cpu->cfg.satp_mode_str, "sv64") && !rv32)
> +cpu->cfg.satp_mode = VM_1_10_SV64;
> +else {
> +error_setg(errp, "Unknown option for satp_mode: %s",
> + cpu->cfg.satp_mode_str);
> +return;
> +}
> +} else {
> +/*
> + * If unset by both the user and the cpu, we fallback to sv32 for
> 32-bit
> + * or sv57 for 64-bit when a MMU is present, and bare otherwise.
> + */
> +if (riscv_feature(>env, RISCV_FEATURE_MMU)) {
> +if (rv32) {
> +cpu->cfg.satp_mode_str = g_strdup("sv32");
> +cpu->cfg.satp_mode = VM_1_10_SV32;
> +} else {
> +cpu->cfg.satp_mode_str = g_strdup("sv57");
> +cpu->cfg.satp_mode = VM_1_10_SV57;
> +}
> +} else {
> +cpu->cfg.satp_mode_str = g_strdup("none");
> +cpu->cfg.satp_mode =
Re: [PATCH for-8.0 01/19] hw/core/cpu-common: Convert TYPE_CPU class to 3-phase reset
On Thu, Nov 24, 2022 at 9:57 PM Peter Maydell wrote:
>
> Convert the parent class TYPE_CPU to 3-phase reset. This
> is a necessary prerequisite to converting the subclasses.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/core/cpu-common.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> index f9fdd46b9d7..78b5f350a00 100644
> --- a/hw/core/cpu-common.c
> +++ b/hw/core/cpu-common.c
> @@ -116,9 +116,9 @@ void cpu_reset(CPUState *cpu)
> trace_guest_cpu_reset(cpu);
> }
>
> -static void cpu_common_reset(DeviceState *dev)
> +static void cpu_common_reset_hold(Object *obj)
> {
> -CPUState *cpu = CPU(dev);
> +CPUState *cpu = CPU(obj);
> CPUClass *cc = CPU_GET_CLASS(cpu);
>
> if (qemu_loglevel_mask(CPU_LOG_RESET)) {
> @@ -259,6 +259,7 @@ static int64_t cpu_common_get_arch_id(CPUState *cpu)
> static void cpu_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> +ResettableClass *rc = RESETTABLE_CLASS(klass);
> CPUClass *k = CPU_CLASS(klass);
>
> k->parse_features = cpu_common_parse_features;
> @@ -269,7 +270,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
> set_bit(DEVICE_CATEGORY_CPU, dc->categories);
> dc->realize = cpu_common_realizefn;
> dc->unrealize = cpu_common_unrealizefn;
> -dc->reset = cpu_common_reset;
> +rc->phases.hold = cpu_common_reset_hold;
> cpu_class_init_props(dc);
> /*
> * Reason: CPUs still need special care by board code: wiring up
> --
> 2.25.1
>
>
Re: [PATCH for-8.0 14/19] target/riscv: Convert to 3-phase reset
On Thu, Nov 24, 2022 at 10:00 PM Peter Maydell wrote:
>
> Convert the riscv CPU class to use 3-phase reset, so it doesn't
> need to use device_class_set_parent_reset() any more.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Alistair Francis
Alistair
> ---
> target/riscv/cpu.h | 4 ++--
> target/riscv/cpu.c | 12
> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3a9e25053f8..443d15a47c0 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -395,7 +395,7 @@ OBJECT_DECLARE_CPU_TYPE(RISCVCPU, RISCVCPUClass,
> RISCV_CPU)
> /**
> * RISCVCPUClass:
> * @parent_realize: The parent class' realize handler.
> - * @parent_reset: The parent class' reset handler.
> + * @parent_phases: The parent class' reset phase handlers.
> *
> * A RISCV CPU model.
> */
> @@ -404,7 +404,7 @@ struct RISCVCPUClass {
> CPUClass parent_class;
> /*< public >*/
> DeviceRealize parent_realize;
> -DeviceReset parent_reset;
> +ResettablePhases parent_phases;
> };
>
> struct RISCVCPUConfig {
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d14e95c9dc1..6fe176e4833 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -519,18 +519,20 @@ static void riscv_restore_state_to_opc(CPUState *cs,
> env->bins = data[1];
> }
>
> -static void riscv_cpu_reset(DeviceState *dev)
> +static void riscv_cpu_reset_hold(Object *obj)
> {
> #ifndef CONFIG_USER_ONLY
> uint8_t iprio;
> int i, irq, rdzero;
> #endif
> -CPUState *cs = CPU(dev);
> +CPUState *cs = CPU(obj);
> RISCVCPU *cpu = RISCV_CPU(cs);
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> CPURISCVState *env = >env;
>
> -mcc->parent_reset(dev);
> +if (mcc->parent_phases.hold) {
> +mcc->parent_phases.hold(obj);
> +}
> #ifndef CONFIG_USER_ONLY
> env->misa_mxl = env->misa_mxl_max;
> env->priv = PRV_M;
> @@ -1161,11 +1163,13 @@ static void riscv_cpu_class_init(ObjectClass *c, void
> *data)
> RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
> CPUClass *cc = CPU_CLASS(c);
> DeviceClass *dc = DEVICE_CLASS(c);
> +ResettableClass *rc = RESETTABLE_CLASS(c);
>
> device_class_set_parent_realize(dc, riscv_cpu_realize,
> >parent_realize);
>
> -device_class_set_parent_reset(dc, riscv_cpu_reset, >parent_reset);
> +resettable_class_set_parent_phases(rc, NULL, riscv_cpu_reset_hold, NULL,
> + >parent_phases);
>
> cc->class_by_name = riscv_cpu_class_by_name;
> cc->has_work = riscv_cpu_has_work;
> --
> 2.25.1
>
>
Re: [PATCH for-8.0 02/19] target/arm: Convert to 3-phase reset
On Thu, Nov 24, 2022 at 9:58 PM Peter Maydell wrote:
>
> Convert the Arm CPU class to use 3-phase reset, so it doesn't
> need to use device_class_set_parent_reset() any more.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Alistair Francis
Alistair
> ---
> target/arm/cpu-qom.h | 4 ++--
> target/arm/cpu.c | 13 +
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
> index 64c44cef2dd..514c22ced9b 100644
> --- a/target/arm/cpu-qom.h
> +++ b/target/arm/cpu-qom.h
> @@ -43,7 +43,7 @@ void aarch64_cpu_register(const ARMCPUInfo *info);
> /**
> * ARMCPUClass:
> * @parent_realize: The parent class' realize handler.
> - * @parent_reset: The parent class' reset handler.
> + * @parent_phases: The parent class' reset phase handlers.
> *
> * An ARM CPU model.
> */
> @@ -54,7 +54,7 @@ struct ARMCPUClass {
>
> const ARMCPUInfo *info;
> DeviceRealize parent_realize;
> -DeviceReset parent_reset;
> +ResettablePhases parent_phases;
> };
>
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index a021df9e9e8..5bad065579f 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -202,14 +202,16 @@ static void cp_reg_check_reset(gpointer key, gpointer
> value, gpointer opaque)
> assert(oldvalue == newvalue);
> }
>
> -static void arm_cpu_reset(DeviceState *dev)
> +static void arm_cpu_reset_hold(Object *obj)
> {
> -CPUState *s = CPU(dev);
> +CPUState *s = CPU(obj);
> ARMCPU *cpu = ARM_CPU(s);
> ARMCPUClass *acc = ARM_CPU_GET_CLASS(cpu);
> CPUARMState *env = >env;
>
> -acc->parent_reset(dev);
> +if (acc->parent_phases.hold) {
> +acc->parent_phases.hold(obj);
> +}
>
> memset(env, 0, offsetof(CPUARMState, end_reset_fields));
>
> @@ -2210,12 +2212,15 @@ static void arm_cpu_class_init(ObjectClass *oc, void
> *data)
> ARMCPUClass *acc = ARM_CPU_CLASS(oc);
> CPUClass *cc = CPU_CLASS(acc);
> DeviceClass *dc = DEVICE_CLASS(oc);
> +ResettableClass *rc = RESETTABLE_CLASS(oc);
>
> device_class_set_parent_realize(dc, arm_cpu_realizefn,
> >parent_realize);
>
> device_class_set_props(dc, arm_cpu_properties);
> -device_class_set_parent_reset(dc, arm_cpu_reset, >parent_reset);
> +
> +resettable_class_set_parent_phases(rc, NULL, arm_cpu_reset_hold, NULL,
> + >parent_phases);
>
> cc->class_by_name = arm_cpu_class_by_name;
> cc->has_work = arm_cpu_has_work;
> --
> 2.25.1
>
>
Re: [PATCH] hw/cxl/cxl-host: Fix an error message typo
On 27/11/22 04:22, Hoa Nguyen wrote: Signed-off-by: Hoa Nguyen --- hw/cxl/cxl-host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 for-7.2 5/6] MAINTAINERS: Add subprojects/libvhost-user to section "vhost"
On Sun, 27 Nov 2022 at 13:09, Stefan Hajnoczi wrote: > > On Sat, 26 Nov 2022 at 10:25, Stefan Weil wrote: > > > > Signed-off-by: Stefan Weil > > --- > > MAINTAINERS | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index cf24910249..6966490c94 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2005,6 +2005,7 @@ F: docs/interop/vhost-user.rst > > F: contrib/vhost-user-*/ > > F: backends/vhost-user.c > > F: include/sysemu/vhost-user-backend.h > > +F: subprojects/libvhost-user/ > > Requires agreement from Michael. Also including Marc-André who > originally created libvhost-user. Stefan showed me the email where Michael agreed: https://lore.kernel.org/qemu-devel/20221123015218-mutt-send-email-...@kernel.org/ Stefan
Re: [PATCH v3 for-7.2 0/6] Add format attributes and fix format strings
On Sun, 27 Nov 2022 at 13:31, Stefan Weil wrote: > > Am 27.11.22 um 19:23 schrieb Stefan Hajnoczi: > > We need to wait for Michael to agree to maintainership in patch 5. If > we run out of time I suggest splitting out patch 5. > > Reviewed-by: Stefan Hajnoczi > > > Citing Michael from a v2 email: "pls do". > > Stefan > > > Hello Michael, > > I just noticed that MAINTAINERS has no entry for the files in > subprojects/libvhost-user, so I did not cc you in my previous e-mails. > Should that directory be added to the "vhost" section"? > > Stefan > > pls do Perfect, thank you! I have added a reference to that email in the commit description. This series is running through CI now and will be merged upon success later today. Thanks, Stefan
Re: [PATCH v3 for-7.2 0/6] Add format attributes and fix format strings
Am 27.11.22 um 19:23 schrieb Stefan Hajnoczi: We need to wait for Michael to agree to maintainership in patch 5. If we run out of time I suggest splitting out patch 5. Reviewed-by: Stefan Hajnoczi Citing Michael from a v2 email: "pls do". Stefan Hello Michael, I just noticed that MAINTAINERS has no entry for the files in subprojects/libvhost-user, so I did not cc you in my previous e-mails. Should that directory be added to the "vhost" section"? Stefan pls do
Re: [PATCH v3 for-7.2 4/6] libvhost-user: Add format attribute to local function vu_panic
Am 27.11.22 um 19:14 schrieb Stefan Hajnoczi: On Sat, 26 Nov 2022 at 10:25, Stefan Weil wrote: Signed-off-by: Stefan Weil Reviewed-by: Marc-André Lureau Message-Id: <20220422070144.1043697-4...@weilnetz.de> Signed-off-by: Laurent Vivier --- subprojects/libvhost-user/libvhost-user.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) I would rather not merge something that can cause new build failures this late in the release cycle. If you respin, please make this a separate 8.0 patch. It would only cause a build failure if there remained more format bugs, but the last ones were fixed in patch 3. I tested a build for 32 bit ARM (which previously failed without patch 3), and it works now. But you are right, patch 4 is not release critical as it is not a bug fix (like the other ones), so not including it in 7.2 might be an option. Stefan
Re: [PATCH v3 for-7.2 0/6] Add format attributes and fix format strings
We need to wait for Michael to agree to maintainership in patch 5. If we run out of time I suggest splitting out patch 5. Reviewed-by: Stefan Hajnoczi
Re: [PATCH v3 for-7.2 4/6] libvhost-user: Add format attribute to local function vu_panic
On Sat, 26 Nov 2022 at 10:25, Stefan Weil wrote:
>
> Signed-off-by: Stefan Weil
> Reviewed-by: Marc-André Lureau
> Message-Id: <20220422070144.1043697-4...@weilnetz.de>
> Signed-off-by: Laurent Vivier
> ---
> subprojects/libvhost-user/libvhost-user.c | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
I would rather not merge something that can cause new build failures
this late in the release cycle.
If you respin, please make this a separate 8.0 patch.
> diff --git a/subprojects/libvhost-user/libvhost-user.c
> b/subprojects/libvhost-user/libvhost-user.c
> index 80f9952e71..d6ee6e7d91 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -45,6 +45,17 @@
> #include "libvhost-user.h"
>
> /* usually provided by GLib */
> +#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ > 4)
> +#if !defined(__clang__) && (__GNUC__ == 4 && __GNUC_MINOR__ == 4)
> +#define G_GNUC_PRINTF(format_idx, arg_idx) \
> + __attribute__((__format__(gnu_printf, format_idx, arg_idx)))
> +#else
> +#define G_GNUC_PRINTF(format_idx, arg_idx) \
> + __attribute__((__format__(__printf__, format_idx, arg_idx)))
> +#endif
> +#else /* !__GNUC__ */
> +#define G_GNUC_PRINTF(format_idx, arg_idx)
> +#endif /* !__GNUC__ */
> #ifndef MIN
> #define MIN(x, y) ({\
> typeof(x) _min1 = (x); \
> @@ -151,7 +162,7 @@ vu_request_to_string(unsigned int req)
> }
> }
>
> -static void
> +static void G_GNUC_PRINTF(2, 3)
> vu_panic(VuDev *dev, const char *msg, ...)
> {
> char *buf = NULL;
> --
> 2.35.1
>
Re: [PATCH v3 for-7.2 5/6] MAINTAINERS: Add subprojects/libvhost-user to section "vhost"
On Sat, 26 Nov 2022 at 10:25, Stefan Weil wrote: > > Signed-off-by: Stefan Weil > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index cf24910249..6966490c94 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2005,6 +2005,7 @@ F: docs/interop/vhost-user.rst > F: contrib/vhost-user-*/ > F: backends/vhost-user.c > F: include/sysemu/vhost-user-backend.h > +F: subprojects/libvhost-user/ Requires agreement from Michael. Also including Marc-André who originally created libvhost-user. Stefan
Re: [PATCH v3 for-7.2 3/6] libvhost-user: Fix two more format strings
On Sat, 26 Nov 2022 at 10:25, Stefan Weil wrote:
>
> This fix is required for 32 bit hosts. The bug was detected by CI
> for arm-linux, but is also relevant for i386-linux.
>
> Reported-by: Stefan Hajnoczi
> Signed-off-by: Stefan Weil
> ---
> subprojects/libvhost-user/libvhost-user.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Stefan Hajnoczi
>
> diff --git a/subprojects/libvhost-user/libvhost-user.c
> b/subprojects/libvhost-user/libvhost-user.c
> index d67953a1c3..80f9952e71 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -651,7 +651,8 @@ generate_faults(VuDev *dev) {
>
> if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, _struct)) {
> vu_panic(dev, "%s: Failed to userfault region %d "
> - "@%" PRIx64 " + size:%zx offset: %zx:
> (ufd=%d)%s\n",
> + "@%" PRIx64 " + size:%" PRIx64 " offset: %" PRIx64
> + ": (ufd=%d)%s\n",
> __func__, i,
> dev_region->mmap_addr,
> dev_region->size, dev_region->mmap_offset,
> --
> 2.35.1
>
Re: [PATCH for 7.2-rc? v2 0/5] continuing efforts to fix vhost-user issues
On Sat, 26 Nov 2022 at 09:18, Alex Bennée wrote:
>
>
> Stefan Hajnoczi writes:
>
> > On Sat, 26 Nov 2022 at 04:45, Alex Bennée wrote:
> >>
> >>
> >> Alex Bennée writes:
> >>
> >> > Alex Bennée writes:
> >> >
> >> >> Hi,
> >> >>
> >> >
> >> >> I can replicate some of the other failures I've been seeing in CI by
> >> >> running:
> >> >>
> >> >> ../../meson/meson.py test --repeat 10 --print-errorlogs
> >> >> qtest-arm/qos-test
> >> >>
> >> >> however this seems to run everything in parallel and maybe is better
> >> >> at exposing race conditions. Perhaps the CI system makes those races
> >> >> easier to hit? Unfortunately I've not been able to figure out exactly
> >> >> how things go wrong in the failure case.
> >> >>
> >> >
> >> >
> >> > There is a circular call - we are in vu_gpio_stop which triggers a write
> >> > to vhost-user which allows us to catch a disconnect event:
> >> >
> >> > #0 vhost_dev_is_started (hdev=0x557adf80d878) at
> >> > /home/alex/lsrc/qemu.git/include/hw/virtio/vhost.h:199
> >> > #1 0x557adbe0518a in vu_gpio_stop (vdev=0x557adf80d640) at
> >> > ../../hw/virtio/vhost-user-gpio.c:138
> >> > #2 0x557adbe04d56 in vu_gpio_disconnect (dev=0x557adf80d640)
> >> > at ../../hw/virtio/vhost-user-gpio.c:255
> >> > #3 0x557adbe049bb in vu_gpio_event (opaque=0x557adf80d640,
> >> > event=CHR_EVENT_CLOSED) at ../../hw/virtio/vhost-user-gpio.c:274
> >>
> >> I suspect the best choice here is to schedule the cleanup as a later
> >> date. Should I use the aio_bh one shots for this or maybe an rcu cleanup
> >> event?
> >>
> >> Paolo, any suggestions?
> >>
> >> > #4 0x557adc0539ef in chr_be_event (s=0x557adea51f10,
> >> > event=CHR_EVENT_CLOSED) at ../../chardev/char.c:61
> >> > #5 0x557adc0506aa in qemu_chr_be_event (s=0x557adea51f10,
> >> > event=CHR_EVENT_CLOSED) at ../../chardev/char.c:81
> >> > #6 0x557adc04f666 in tcp_chr_disconnect_locked
> >> > (chr=0x557adea51f10) at ../../chardev/char-socket.c:470
> >> > #7 0x557adc04c81a in tcp_chr_write (chr=0x557adea51f10,
> >> > buf=0x7ffe8588cce0 "\v", len=20) at
> >> > ../../chardev/char-socket.c:129
> >
> > Does this mean the backend closed the connection before receiving all
> > the vhost-user protocol messages sent by QEMU?
> >
> > This looks like a backend bug. It prevents QEMU's vhost-user client
> > from cleanly stopping the virtqueue (vhost_virtqueue_stop()).
>
> Well the backend in this case is the qtest framework so not the worlds
> most complete implementation.
>
> > QEMU is still broken if it cannot handle disconnect at any time. Maybe
> > a simple solution for that is to check for reentrancy (either by
> > checking an existing variable or adding a new one to prevent
> > vu_gpio_stop() from calling itself).
>
> vhost-user-blk introduced an additional flag:
>
> /*
> * There are at least two steps of initialization of the
> * vhost-user device. The first is a "connect" step and
> * second is a "start" step. Make a separation between
> * those initialization phases by using two fields.
> */
> /* vhost_user_blk_connect/vhost_user_blk_disconnect */
> bool connected;
> /* vhost_user_blk_start/vhost_user_blk_stop */
> bool started_vu;
>
> but that in itself is not enough. If you look at the various cases of
> handling CHR_EVENT_CLOSED you'll see some schedule the shutdown with aio
> and some don't even bother (so will probably break the same way).
>
> Rather than have a mish-mash of solutions maybe we should introduce a
> new vhost function - vhost_user_async_close() which can take care of the
> scheduling and wrap it with a check for a valid vhost structure in case
> it gets shutdown in the meantime?
Handling this in core vhost code would be great.
I suggested checking a variable because it's not async. Async is more
complicated because it creates a new in-between state while waiting
for the operation to complete. Async approaches are more likely to
have bugs for this reason.
vhost-user-blk.c's async shutdown is a good example of that:
case CHR_EVENT_CLOSED:
if (!runstate_check(RUN_STATE_SHUTDOWN)) {
/*
* A close event may happen during a read/write, but vhost
* code assumes the vhost_dev remains setup, so delay the
* stop & clear.
*/
AioContext *ctx = qemu_get_current_aio_context();
qemu_chr_fe_set_handlers(>chardev, NULL, NULL, NULL, NULL,
NULL, NULL, false);
aio_bh_schedule_oneshot(ctx, vhost_user_blk_chr_closed_bh, opaque);
/*
* Move vhost device to the stopped state. The vhost-user device
* will be clean up and disconnected in BH. This can be useful in
* the vhost migration code. If disconnect was caught there is an
* option for the general vhost code to get the dev state without
* knowing its type (in this case vhost-user).
Re: [PATCH] tests/qtest/migration-test: Fix unlink error and memory leaks
Thomas Huth wrote: > When running the migration test compiled with Clang from Fedora 37 > and sanitizers enabled, there is an error complaining about unlink(): > > ../tests/qtest/migration-test.c:1072:12: runtime error: null pointer > passed as argument 1, which is declared to never be null > /usr/include/unistd.h:858:48: note: nonnull attribute specified here > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > ../tests/qtest/migration-test.c:1072:12 in > (test program exited with status code 1) > TAP parsing error: Too few tests run (expected 33, got 20) > > The data->clientcert and data->clientkey pointers can indeed be unset > in some tests, so we have to check them before calling unlink() with > those. > > While we're at it, I also noticed that the code is only freeing > some but not all of the allocated strings in this function, and > indeed, valgrind is also complaining about memory leaks here. > So let's call g_free() on all allocated strings to avoid leaking > memory here. > > Signed-off-by: Thomas Huth Reviewed-by: Juan Quintela
[PATCH] hw/intc: sifive_plic: fix out-of-bound access of source_priority array
If the number of interrupt is not multiple of 32, PLIC will have
out-of-bound access to source_priority array. Compute the number of
interrupt in the last word to avoid this out-of-bound access of array.
Signed-off-by: Jim Shu
---
hw/intc/sifive_plic.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index c2dfacf028..1cf156cf85 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -78,6 +78,7 @@ static uint32_t sifive_plic_claimed(SiFivePLICState *plic,
uint32_t addrid)
uint32_t max_irq = 0;
uint32_t max_prio = plic->target_priority[addrid];
int i, j;
+int num_irq_in_word = 32;
for (i = 0; i < plic->bitfield_words; i++) {
uint32_t pending_enabled_not_claimed =
@@ -88,7 +89,16 @@ static uint32_t sifive_plic_claimed(SiFivePLICState *plic,
uint32_t addrid)
continue;
}
-for (j = 0; j < 32; j++) {
+if (i == (plic->bitfield_words - 1)) {
+/*
+ * If plic->num_sources is not multiple of 32, num-of-irq in last
+ * word is not 32. Compute the num-of-irq of last word to avoid
+ * out-of-bound access of source_priority array.
+ */
+num_irq_in_word = plic->num_sources - ((plic->bitfield_words - 1)
<< 5);
+}
+
+for (j = 0; j < num_irq_in_word; j++) {
int irq = (i << 5) + j;
uint32_t prio = plic->source_priority[irq];
int enabled = pending_enabled_not_claimed & (1 << j);
--
2.17.1
[PATCH] hw/cxl/cxl-host: Fix an error message typo
Signed-off-by: Hoa Nguyen
---
hw/cxl/cxl-host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 1adf61231a..3c1ec8732a 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -47,7 +47,7 @@ static void cxl_fixed_memory_window_config(CXLState
*cxl_state,
if (object->size % (256 * MiB)) {
error_setg(errp,
- "Size of a CXL fixed memory window must my a multiple of
256MiB");
+ "Size of a CXL fixed memory window must be a multiple of
256MiB");
return;
}
fw->size = object->size;
--
2.30.2
[PATCH v5 4/7] target/arm: Enable TTBCR_EAE for ARMv8-R AArch32
From: Tobias Röhmel
ARMv8-R AArch32 CPUs behave as if TTBCR.EAE is always 1 even
tough they don't have the TTBCR register.
See ARM Architecture Reference Manual Supplement - ARMv8, for the ARMv8-R
AArch32 architecture profile Version:A.c section C1.2.
Signed-off-by: Tobias Röhmel
---
target/arm/debug_helper.c | 3 +++
target/arm/internals.h| 4
target/arm/tlb_helper.c | 4
3 files changed, 11 insertions(+)
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index c21739242c..2f6ddc0da5 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -437,6 +437,9 @@ static uint32_t arm_debug_exception_fsr(CPUARMState *env)
if (target_el == 2 || arm_el_is_aa64(env, target_el)) {
using_lpae = true;
+} else if (arm_feature(env, ARM_FEATURE_PMSA) &&
+ arm_feature(env, ARM_FEATURE_V8)) {
+using_lpae = true;
} else {
if (arm_feature(env, ARM_FEATURE_LPAE) &&
(env->cp15.tcr_el[target_el] & TTBCR_EAE)) {
diff --git a/target/arm/internals.h b/target/arm/internals.h
index d9121d9ff8..1dfc593f28 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -253,6 +253,10 @@ unsigned int arm_pamax(ARMCPU *cpu);
static inline bool extended_addresses_enabled(CPUARMState *env)
{
uint64_t tcr = env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1];
+if (arm_feature(env, ARM_FEATURE_PMSA) &&
+arm_feature(env, ARM_FEATURE_V8)) {
+return true;
+}
return arm_el_is_aa64(env, 1) ||
(arm_feature(env, ARM_FEATURE_LPAE) && (tcr & TTBCR_EAE));
}
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 0f4f4fc809..60abcbebe6 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -19,6 +19,10 @@ bool regime_using_lpae_format(CPUARMState *env, ARMMMUIdx
mmu_idx)
if (el == 2 || arm_el_is_aa64(env, el)) {
return true;
}
+if (arm_feature(env, ARM_FEATURE_PMSA) &&
+arm_feature(env, ARM_FEATURE_V8)) {
+return true;
+}
if (arm_feature(env, ARM_FEATURE_LPAE)
&& (regime_tcr(env, mmu_idx) & TTBCR_EAE)) {
return true;
--
2.34.1
[PATCH v5 5/7] target/arm: Add PMSAv8r registers
From: Tobias Röhmel
Signed-off-by: Tobias Röhmel
---
target/arm/cpu.c | 24 +++-
target/arm/cpu.h | 6 +
target/arm/helper.c | 299 +++
target/arm/machine.c | 28
4 files changed, 356 insertions(+), 1 deletion(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index cccd957553..1df625783d 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -489,6 +489,14 @@ static void arm_cpu_reset(DeviceState *dev)
sizeof(*env->pmsav7.dracr) * cpu->pmsav7_dregion);
}
}
+
+if (cpu->pmsav8r_hdregion > 0) {
+memset(env->pmsav8.hprbar, 0,
+ sizeof(*env->pmsav8.hprbar) * cpu->pmsav8r_hdregion);
+memset(env->pmsav8.hprlar, 0,
+ sizeof(*env->pmsav8.hprlar) * cpu->pmsav8r_hdregion);
+}
+
env->pmsav7.rnr[M_REG_NS] = 0;
env->pmsav7.rnr[M_REG_S] = 0;
env->pmsav8.mair0[M_REG_NS] = 0;
@@ -2001,8 +2009,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
*/
if (!cpu->has_mpu) {
cpu->pmsav7_dregion = 0;
+cpu->pmsav8r_hdregion = 0;
}
-if (cpu->pmsav7_dregion == 0) {
+if ((cpu->pmsav7_dregion == 0) && (cpu->pmsav8r_hdregion == 0)) {
cpu->has_mpu = false;
}
@@ -2030,6 +2039,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
env->pmsav7.dracr = g_new0(uint32_t, nr);
}
}
+
+if (cpu->pmsav8r_hdregion > 0xFF) {
+error_setg(errp, "PMSAv8 MPU EL2 #regions invalid %" PRIu32,
+ cpu->pmsav8r_hdregion);
+return;
+}
+
+if (cpu->pmsav8r_hdregion) {
+env->pmsav8.hprbar = g_new0(uint32_t,
+cpu->pmsav8r_hdregion);
+env->pmsav8.hprlar = g_new0(uint32_t,
+cpu->pmsav8r_hdregion);
+}
}
if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 9aeed3c848..c2eab52174 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -309,6 +309,7 @@ typedef struct CPUArchState {
};
uint64_t sctlr_el[4];
};
+uint64_t vsctlr; /* Virtualization System control register. */
uint64_t cpacr_el1; /* Architectural feature access control register */
uint64_t cptr_el[4]; /* ARMv8 feature trap registers */
uint32_t c1_xscaleauxcr; /* XScale auxiliary control register. */
@@ -745,8 +746,11 @@ typedef struct CPUArchState {
*/
uint32_t *rbar[M_REG_NUM_BANKS];
uint32_t *rlar[M_REG_NUM_BANKS];
+uint32_t *hprbar;
+uint32_t *hprlar;
uint32_t mair0[M_REG_NUM_BANKS];
uint32_t mair1[M_REG_NUM_BANKS];
+uint32_t hprselr;
} pmsav8;
/* v8M SAU */
@@ -906,6 +910,8 @@ struct ArchCPU {
bool has_mpu;
/* PMSAv7 MPU number of supported regions */
uint32_t pmsav7_dregion;
+/* PMSAv8 MPU number of supported hyp regions */
+uint32_t pmsav8r_hdregion;
/* v8M SAU number of supported regions */
uint32_t sau_sregion;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 23a55dbe7d..2d4d110644 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3657,6 +3657,222 @@ static void pmsav7_rgnr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
raw_write(env, ri, value);
}
+static void prbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ARMCPU *cpu = env_archcpu(env);
+
+tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
+env->pmsav8.rbar[M_REG_NS][env->pmsav7.rnr[M_REG_NS]] = value;
+}
+
+static uint64_t prbar_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+return env->pmsav8.rbar[M_REG_NS][env->pmsav7.rnr[M_REG_NS]];
+}
+
+static void prlar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ARMCPU *cpu = env_archcpu(env);
+
+tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
+env->pmsav8.rlar[M_REG_NS][env->pmsav7.rnr[M_REG_NS]] = value;
+}
+
+static uint64_t prlar_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+return env->pmsav8.rlar[M_REG_NS][env->pmsav7.rnr[M_REG_NS]];
+}
+
+static void prselr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ARMCPU *cpu = env_archcpu(env);
+
+/*
+ * Ignore writes that would select not implemented region.
+ * This is architecturally UNPREDICTABLE.
+ */
+if (value >= cpu->pmsav7_dregion) {
+return;
+}
+
+env->pmsav7.rnr[M_REG_NS] = value;
+}
+
+static void hprbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ARMCPU *cpu = env_archcpu(env);
+
+tlb_flush(CPU(cpu)); /* Mappings may have changed -
[PATCH v5 3/7] target/arm: Make stage_2_format for cache attributes optional
From: Tobias Röhmel
The v8R PMSAv8 has a two-stage MPU translation process, but, unlike
VMSAv8, the stage 2 attributes are in the same format as the stage 1
attributes (8-bit MAIR format). Rather than converting the MAIR
format to the format used for VMSA stage 2 (bits [5:2] of a VMSA
stage 2 descriptor) and then converting back to do the attribute
combination, allow combined_attrs_nofwb() to accept s2 attributes
that are already in the MAIR format.
We move the assert() to combined_attrs_fwb(), because that function
really does require a VMSA stage 2 attribute format. (We will never
get there for v8R, because PMSAv8 does not implement FEAT_S2FWB.)
Signed-off-by: Tobias Röhmel
---
target/arm/ptw.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index f812734bfb..7d19829702 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2361,7 +2361,11 @@ static uint8_t combined_attrs_nofwb(uint64_t hcr,
{
uint8_t s1lo, s2lo, s1hi, s2hi, s2_mair_attrs, ret_attrs;
-s2_mair_attrs = convert_stage2_attrs(hcr, s2.attrs);
+if (s2.is_s2_format) {
+s2_mair_attrs = convert_stage2_attrs(hcr, s2.attrs);
+} else {
+s2_mair_attrs = s2.attrs;
+}
s1lo = extract32(s1.attrs, 0, 4);
s2lo = extract32(s2_mair_attrs, 0, 4);
@@ -2418,6 +2422,8 @@ static uint8_t force_cacheattr_nibble_wb(uint8_t attr)
*/
static uint8_t combined_attrs_fwb(ARMCacheAttrs s1, ARMCacheAttrs s2)
{
+assert(s2.is_s2_format && !s1.is_s2_format);
+
switch (s2.attrs) {
case 7:
/* Use stage 1 attributes */
@@ -2467,7 +2473,7 @@ static ARMCacheAttrs combine_cacheattrs(uint64_t hcr,
ARMCacheAttrs ret;
bool tagged = false;
-assert(s2.is_s2_format && !s1.is_s2_format);
+assert(!s1.is_s2_format);
ret.is_s2_format = false;
if (s1.attrs == 0xf0) {
--
2.34.1
[PATCH v5 0/7] Add ARM Cortex-R52 CPU
From: Tobias Röhmel No worries about the delay. I'm glad you are looking at it :) v5: 1. Adjusted the spacing as requested 2. Removed cp 15 3. Rebased and put assert back 4. Fixed indention issues 5. - Made hprbar etc pointers instead of arrays - Fixed the logic/bound issues - For the VMSTATE change I looked at pmsav7.drbar which is a pointer and is handled as an array. I assume this works for hprbar/hprlar 6. - In pmsav7_use_background_region there are 2 cases were we don't want to look at the SCTLR_BR bit (c1.3 in manual supplement): - The respective MPU is enabled and - We are in the second translation stage - We are in EL0 I think the code does that now and doesn't influence any other code. I put the V8 check in there because the function is also called from get_phys_addr_pmsav7 - I put the fi->level behaviour back the way it was - Fixed UWXN/WXN Tobias Röhmel (7): target/arm: Don't add all MIDR aliases for cores that implement PMSA target/arm: Make RVBAR available for all ARMv8 CPUs target/arm: Make stage_2_format for cache attributes optional target/arm: Enable TTBCR_EAE for ARMv8-R AArch32 target/arm: Add PMSAv8r registers target/arm: Add PMSAv8r functionality target/arm: Add ARM Cortex-R52 CPU target/arm/cpu.c | 30 +++- target/arm/cpu.h | 6 + target/arm/cpu_tcg.c | 42 + target/arm/debug_helper.c | 3 + target/arm/helper.c | 333 -- target/arm/internals.h| 4 + target/arm/machine.c | 28 target/arm/ptw.c | 137 +--- target/arm/tlb_helper.c | 4 + 9 files changed, 550 insertions(+), 37 deletions(-) -- 2.34.1
[PATCH v5 1/7] target/arm: Don't add all MIDR aliases for cores that implement PMSA
From: Tobias Röhmel
Cores with PMSA have the MPUIR register which has the
same encoding as the MIDR alias with opc2=4. So we only
add that alias if we are not realizing a core that
implements PMSA.
Signed-off-by: Tobias Röhmel
---
target/arm/helper.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d8c8223ec3..d857d61fa9 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8112,10 +8112,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.access = PL1_R, .type = ARM_CP_NO_RAW, .resetvalue = cpu->midr,
.fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid),
.readfn = midr_read },
-/* crn = 0 op1 = 0 crm = 0 op2 = 4,7 : AArch32 aliases of MIDR */
-{ .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
- .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
- .access = PL1_R, .resetvalue = cpu->midr },
+/* crn = 0 op1 = 0 crm = 0 op2 = 7 : AArch32 aliases of MIDR */
{ .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
.cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 7,
.access = PL1_R, .resetvalue = cpu->midr },
@@ -8125,6 +8122,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.accessfn = access_aa64_tid1,
.type = ARM_CP_CONST, .resetvalue = cpu->revidr },
};
+ARMCPRegInfo id_v8_midr_alias_cp_reginfo = {
+.name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
+.cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
+.access = PL1_R, .resetvalue = cpu->midr
+};
ARMCPRegInfo id_cp_reginfo[] = {
/* These are common to v8 and pre-v8 */
{ .name = "CTR",
@@ -8190,6 +8192,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
}
if (arm_feature(env, ARM_FEATURE_V8)) {
define_arm_cp_regs(cpu, id_v8_midr_cp_reginfo);
+if (!arm_feature(env, ARM_FEATURE_PMSA)) {
+define_one_arm_cp_reg(cpu, _v8_midr_alias_cp_reginfo);
+}
} else {
define_arm_cp_regs(cpu, id_pre_v8_midr_cp_reginfo);
}
--
2.34.1
[PATCH v5 2/7] target/arm: Make RVBAR available for all ARMv8 CPUs
From: Tobias Röhmel
RVBAR shadows RVBAR_ELx where x is the highest exception
level if the highest EL is not EL3. This patch also allows
ARMv8 CPUs to change the reset address with
the rvbar property.
Signed-off-by: Tobias Röhmel
---
target/arm/cpu.c| 6 +-
target/arm/helper.c | 21 ++---
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a021df9e9e..cccd957553 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -307,6 +307,10 @@ static void arm_cpu_reset(DeviceState *dev)
env->cp15.cpacr_el1 = FIELD_DP64(env->cp15.cpacr_el1,
CPACR, CP11, 3);
#endif
+if (arm_feature(env, ARM_FEATURE_V8)) {
+env->cp15.rvbar = cpu->rvbar_prop;
+env->regs[15] = cpu->rvbar_prop;
+}
}
#if defined(CONFIG_USER_ONLY)
@@ -1342,7 +1346,7 @@ void arm_cpu_post_init(Object *obj)
qdev_property_add_static(DEVICE(obj), _cpu_reset_hivecs_property);
}
-if (arm_feature(>env, ARM_FEATURE_AARCH64)) {
+if (arm_feature(>env, ARM_FEATURE_V8)) {
object_property_add_uint64_ptr(obj, "rvbar",
>rvbar_prop,
OBJ_PROP_FLAG_READWRITE);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d857d61fa9..23a55dbe7d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7855,7 +7855,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
if (!arm_feature(env, ARM_FEATURE_EL3) &&
!arm_feature(env, ARM_FEATURE_EL2)) {
ARMCPRegInfo rvbar = {
-.name = "RVBAR_EL1", .state = ARM_CP_STATE_AA64,
+.name = "RVBAR_EL1", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 1,
.access = PL1_R,
.fieldoffset = offsetof(CPUARMState, cp15.rvbar),
@@ -7946,13 +7946,20 @@ void register_cp_regs_for_features(ARMCPU *cpu)
}
/* RVBAR_EL2 is only implemented if EL2 is the highest EL */
if (!arm_feature(env, ARM_FEATURE_EL3)) {
-ARMCPRegInfo rvbar = {
-.name = "RVBAR_EL2", .state = ARM_CP_STATE_AA64,
-.opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 1,
-.access = PL2_R,
-.fieldoffset = offsetof(CPUARMState, cp15.rvbar),
+ARMCPRegInfo rvbar[] = {
+{
+.name = "RVBAR_EL2", .state = ARM_CP_STATE_AA64,
+.opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 1,
+.access = PL2_R,
+.fieldoffset = offsetof(CPUARMState, cp15.rvbar),
+},
+{ .name = "RVBAR", .type = ARM_CP_ALIAS,
+.cp = 15, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 1,
+.access = PL2_R,
+.fieldoffset = offsetof(CPUARMState, cp15.rvbar),
+},
};
-define_one_arm_cp_reg(cpu, );
+define_arm_cp_regs(cpu, rvbar);
}
}
--
2.34.1
[PATCH v5 7/7] target/arm: Add ARM Cortex-R52 CPU
From: Tobias Röhmel
All constants are taken from the ARM Cortex-R52 Processor TRM Revision: r1p3
Signed-off-by: Tobias Röhmel
---
target/arm/cpu_tcg.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 60ff539fa1..ae08322758 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -853,6 +853,47 @@ static void cortex_r5_initfn(Object *obj)
define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
}
+static void cortex_r52_initfn(Object *obj)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+
+set_feature(>env, ARM_FEATURE_V8);
+set_feature(>env, ARM_FEATURE_EL2);
+set_feature(>env, ARM_FEATURE_PMSA);
+set_feature(>env, ARM_FEATURE_NEON);
+set_feature(>env, ARM_FEATURE_GENERIC_TIMER);
+cpu->midr = 0x411fd133; /* r1p3 */
+cpu->revidr = 0x;
+cpu->reset_fpsid = 0x41034023;
+cpu->isar.mvfr0 = 0x10110222;
+cpu->isar.mvfr1 = 0x1211;
+cpu->isar.mvfr2 = 0x0043;
+cpu->ctr = 0x8144c004;
+cpu->reset_sctlr = 0x30c50838;
+cpu->isar.id_pfr0 = 0x0131;
+cpu->isar.id_pfr1 = 0x10111001;
+cpu->isar.id_dfr0 = 0x03010006;
+cpu->id_afr0 = 0x;
+cpu->isar.id_mmfr0 = 0x00211040;
+cpu->isar.id_mmfr1 = 0x4000;
+cpu->isar.id_mmfr2 = 0x0120;
+cpu->isar.id_mmfr3 = 0xf0102211;
+cpu->isar.id_mmfr4 = 0x0010;
+cpu->isar.id_isar0 = 0x02101110;
+cpu->isar.id_isar1 = 0x13112111;
+cpu->isar.id_isar2 = 0x21232142;
+cpu->isar.id_isar3 = 0x01112131;
+cpu->isar.id_isar4 = 0x00010142;
+cpu->isar.id_isar5 = 0x00010001;
+cpu->isar.dbgdidr = 0x77168000;
+cpu->clidr = (1 << 27) | (1 << 24) | 0x3;
+cpu->ccsidr[0] = 0x700fe01a; /* 32KB L1 dcache */
+cpu->ccsidr[1] = 0x201fe00a; /* 32KB L1 icache */
+
+cpu->pmsav7_dregion = 16;
+cpu->pmsav8r_hdregion = 16;
+}
+
static void cortex_r5f_initfn(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
@@ -1161,6 +1202,7 @@ static const ARMCPUInfo arm_tcg_cpus[] = {
.class_init = arm_v7m_class_init },
{ .name = "cortex-r5", .initfn = cortex_r5_initfn },
{ .name = "cortex-r5f", .initfn = cortex_r5f_initfn },
+{ .name = "cortex-r52", .initfn = cortex_r52_initfn },
{ .name = "ti925t", .initfn = ti925t_initfn },
{ .name = "sa1100", .initfn = sa1100_initfn },
{ .name = "sa1110", .initfn = sa1110_initfn },
--
2.34.1
[PATCH v5 6/7] target/arm: Add PMSAv8r functionality
From: Tobias Röhmel
Add PMSAv8r translation.
Signed-off-by: Tobias Röhmel
---
target/arm/ptw.c | 127 +++
1 file changed, 105 insertions(+), 22 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 7d19829702..0514a83c1b 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1758,9 +1758,14 @@ static bool pmsav7_use_background_region(ARMCPU *cpu,
ARMMMUIdx mmu_idx,
if (arm_feature(env, ARM_FEATURE_M)) {
return env->v7m.mpu_ctrl[is_secure] & R_V7M_MPU_CTRL_PRIVDEFENA_MASK;
-} else {
-return regime_sctlr(env, mmu_idx) & SCTLR_BR;
}
+
+if (arm_feature(env, ARM_FEATURE_V8) &&
+((mmu_idx == ARMMMUIdx_Stage2) || (mmu_idx == ARMMMUIdx_Stage1_E0))) {
+return false;
+}
+
+return regime_sctlr(env, mmu_idx) & SCTLR_BR;
}
static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
@@ -1952,6 +1957,26 @@ static bool get_phys_addr_pmsav7(CPUARMState *env,
uint32_t address,
return !(result->f.prot & (1 << access_type));
}
+static uint32_t *regime_rbar(CPUARMState *env, ARMMMUIdx mmu_idx,
+ uint32_t secure)
+{
+if (regime_el(env, mmu_idx) == 2) {
+return env->pmsav8.hprbar;
+} else {
+return env->pmsav8.rbar[secure];
+}
+}
+
+static uint32_t *regime_rlar(CPUARMState *env, ARMMMUIdx mmu_idx,
+ uint32_t secure)
+{
+if (regime_el(env, mmu_idx) == 2) {
+return env->pmsav8.hprlar;
+} else {
+return env->pmsav8.rlar[secure];
+}
+}
+
bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
MMUAccessType access_type, ARMMMUIdx mmu_idx,
bool secure, GetPhysAddrResult *result,
@@ -1974,6 +1999,13 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t
address,
bool hit = false;
uint32_t addr_page_base = address & TARGET_PAGE_MASK;
uint32_t addr_page_limit = addr_page_base + (TARGET_PAGE_SIZE - 1);
+int region_counter;
+
+if (regime_el(env, mmu_idx) == 2) {
+region_counter = cpu->pmsav8r_hdregion;
+} else {
+region_counter = cpu->pmsav7_dregion;
+}
result->f.lg_page_size = TARGET_PAGE_BITS;
result->f.phys_addr = address;
@@ -1982,6 +2014,10 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t
address,
*mregion = -1;
}
+if (mmu_idx == ARMMMUIdx_Stage2) {
+fi->stage2 = true;
+}
+
/*
* Unlike the ARM ARM pseudocode, we don't need to check whether this
* was an exception vector read from the vector table (which is always
@@ -1998,17 +2034,26 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t
address,
hit = true;
}
-for (n = (int)cpu->pmsav7_dregion - 1; n >= 0; n--) {
+uint32_t bitmask;
+if (arm_feature(env, ARM_FEATURE_M)) {
+bitmask = 0x1f;
+} else {
+bitmask = 0x3f;
+fi->level = 0;
+}
+
+for (n = region_counter - 1; n >= 0; n--) {
/* region search */
/*
- * Note that the base address is bits [31:5] from the register
- * with bits [4:0] all zeroes, but the limit address is bits
- * [31:5] from the register with bits [4:0] all ones.
+ * Note that the base address is bits [31:x] from the register
+ * with bits [x-1:0] all zeroes, but the limit address is bits
+ * [31:x] from the register with bits [x:0] all ones. Where x is
+ * 5 for Cortex-M and 6 for Cortex-R
*/
-uint32_t base = env->pmsav8.rbar[secure][n] & ~0x1f;
-uint32_t limit = env->pmsav8.rlar[secure][n] | 0x1f;
+uint32_t base = regime_rbar(env, mmu_idx, secure)[n] & ~bitmask;
+uint32_t limit = regime_rlar(env, mmu_idx, secure)[n] | bitmask;
-if (!(env->pmsav8.rlar[secure][n] & 0x1)) {
+if (!(regime_rlar(env, mmu_idx, secure)[n] & 0x1)) {
/* Region disabled */
continue;
}
@@ -2042,7 +2087,9 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
* PMSAv7 where highest-numbered-region wins)
*/
fi->type = ARMFault_Permission;
-fi->level = 1;
+if (arm_feature(env, ARM_FEATURE_M)) {
+fi->level = 1;
+}
return true;
}
@@ -2052,8 +2099,11 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t
address,
}
if (!hit) {
-/* background fault */
-fi->type = ARMFault_Background;
+if (arm_feature(env, ARM_FEATURE_M)) {
+fi->type = ARMFault_Background;
+} else {
+fi->type = ARMFault_Permission;
+}
return true;
}
@@ -2061,12 +2111,14 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t
Re: [PATCH v9 00/10] s390x: CPU Topology
On 11/24/22 10:25, Pierre Morel wrote: Gentle ping. Did I understand the problem or am I wrong? I guess I was wrong, so I send a new series next week. Regards, Pierre On 11/17/22 17:38, Pierre Morel wrote: On 11/17/22 10:31, Pierre Morel wrote: On 11/16/22 17:51, Christian Borntraeger wrote: Am 02.09.22 um 09:55 schrieb Pierre Morel: Hi, The implementation of the CPU Topology in QEMU has been drastically modified since the last patch series and the number of LOCs has been greatly reduced. Unnecessary objects have been removed, only a single S390Topology object is created to support migration and reset. Also a documentation has been added to the series. To use these patches, you will need Linux V6-rc1 or newer. Mainline patches needed are: f5ecfee94493 2022-07-20 KVM: s390: resetting the Topology-Change-Report 24fe0195bc19 2022-07-20 KVM: s390: guest support for topology function 0130337ec45b 2022-07-20 KVM: s390: Cleanup ipte lock access and SIIF fac.. Currently this code is for KVM only, I have no idea if it is interesting to provide a TCG patch. If ever it will be done in another series. To have a better understanding of the S390x CPU Topology and its implementation in QEMU you can have a look at the documentation in the last patch. New in this series == s390x/cpus: Make absence of multithreading clear This patch makes clear that CPU-multithreading is not supported in the guest. s390x/cpu topology: core_id sets s390x CPU topology This patch uses the core_id to build the container topology and the placement of the CPU inside the container. s390x/cpu topology: reporting the CPU topology to the guest This patch is based on the fact that the CPU type for guests is always IFL, CPUs are always dedicated and the polarity is always horizontal. This may change in the future. hw/core: introducing drawer and books for s390x s390x/cpu: reporting drawers and books topology to the guest These two patches extend the topology handling to add two new containers levels above sockets: books and drawers. The subject of the last patches is clear enough (I hope). Regards, Pierre Pierre Morel (10): s390x/cpus: Make absence of multithreading clear s390x/cpu topology: core_id sets s390x CPU topology s390x/cpu topology: reporting the CPU topology to the guest hw/core: introducing drawer and books for s390x s390x/cpu: reporting drawers and books topology to the guest s390x/cpu_topology: resetting the Topology-Change-Report s390x/cpu_topology: CPU topology migration target/s390x: interception of PTF instruction s390x/cpu_topology: activating CPU topology Do we really need a machine property? As far as I can see, old QEMU cannot activate the ctop facility with old and new kernel unless it enables CAP_S390_CPU_TOPOLOGY. I do get oldqemu -cpu z14,ctop=on qemu-system-s390x: Some features requested in the CPU model are not available in the configuration: ctop With the newer QEMU we can. So maybe we can simply have a topology (and then a cpu model feature) in new QEMUs and non in old. the cpu model would then also fence migration from enabled to disabled. OK, I can check this. In this case migration with topology will be if I understand correctly: NEW_QEMU/old_machine <-> NEW_QEMU/old_machine OK While OLD_QEMU/old_machine <-> NEW_QEMU/old_machine KO NEW_QEMU/old_machine <-> OLD_QEMU/old_machine KO I forgot to say that I mean in the examples above without using a flag. Of course using a flag like -ctop=off in NEW_QEMU/new_machine allows to migrate from and to old_machines in an old QEMU. Also I had the same behavior already in V9 by having a VMState without the creation of a machine property, a new cpu feature and a new cpu flag. Is this something we can accept? regards, Pierre -- Pierre Morel IBM Lab Boeblingen
Re: [PATCH v11 04/11] s390x/cpu topology: reporting the CPU topology to the guest
On 11/22/22 10:05, Pierre Morel wrote:
On 11/21/22 15:13, Cédric Le Goater wrote:
+static char *s390_top_set_level2(S390Topology *topo, char *p)
+{
+ int i, origin;
+
+ for (i = 0; i < topo->nr_sockets; i++) {
+ if (!topo->socket[i].active_count) {
+ continue;
+ }
+ p = fill_container(p, 1, i);
+ for (origin = 0; origin < S390_TOPOLOGY_MAX_ORIGIN;
origin++) {
+ uint64_t mask = 0L;
+
+ mask = topo->socket[i].mask[origin];
+ if (mask) {
+ p = fill_tle_cpu(p, mask, origin);
+ }
+ }
+ }
+ return p;
+}
Why is it not possible to compute this topo information at "runtime",
when stsi is called, without maintaining state in an extra S390Topology
object ? Couldn't we loop on the CPU list to gather the topology bits
for the same result ?
It would greatly simplify the feature.
C.
The vCPU are not stored in order of creation in the CPU list and not
in a topology order.
To be able to build the SYSIB we need an intermediate structure to
reorder the CPUs per container.
We can do this re-ordering during the STSI interception but the idea
was to keep this instruction as fast as possible.> The second reason
is to have a structure ready for the QEMU migration when we introduce
vCPU migration from a socket to another socket, having then a
different internal representation of the topology.
However, if as discussed yesterday we use a new cpu flag we would not
need any special migration structure in the current series.
So it only stays the first reason to do the re-ordering preparation
during the plugging of a vCPU, to optimize the STSI instruction.
If we think the optimization is not worth it or do not bring enough
to be consider, we can do everything during the STSI interception.
Is it called on a hot code path ? AFAICT, it is only called once
per cpu when started. insert_stsi_3_2_2 is also a guest exit andit
queries the machine definition in a very similar way.
It is not fully exact, stsi(15) is called at several moments, not only
on CPU creation, but each time the core calls rebuild_sched_domains()
that is for s390 on:
- change in the host topology
- changes in CPUSET: for allowed CPU or load balancing
Regards,
Pierre
These are no good reasons to not make as you propose.
This allows to use the s390_has_feature() and use the cpu feature as
proposed Christian.
What I can not do with the early topology initialization.
Regards,
Pierre
Thanks,
C.
--
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v2 2/5] include/hw: VM state takes precedence in virtio_device_should_start
Am 25. November 2022 17:30:40 UTC schrieb "Alex Bennée"
:
>The VM status should always preempt the device status for these
>checks. This ensures the device is in the correct state when we
>suspend the VM prior to migrations. This restores the checks to the
>order they where in before the refactoring moved things around.
>
>While we are at it lets improve our documentation of the various
>fields involved and document the two functions.
>
>Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started)
>Fixes: 259d69c00b (hw/virtio: introduce virtio_device_should_start)
>Signed-off-by: Alex Bennée
>Tested-by: Christian Borntraeger
>---
> include/hw/virtio/virtio.h | 24 +++-
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
>diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>index 0f612067f7..48f539d0fe 100644
>--- a/include/hw/virtio/virtio.h
>+++ b/include/hw/virtio/virtio.h
>@@ -133,6 +133,13 @@ struct VirtIODevice
> bool broken; /* device in invalid state, needs reset */
> bool use_disabled_flag; /* allow use of 'disable' flag when needed */
> bool disabled; /* device in temporarily disabled state */
>+/**
>+ * @use_started: true if the @started flag should be used to check the
>+ * current state of the VirtIO device. Otherwise status bits
>+ * should be checked for a current status of the device.
>+ * @use_started is only set via QMP and defaults to true for all
>+ * modern machines (since 4.1).
>+ */
> bool use_started;
> bool started;
> bool start_on_kick; /* when virtio 1.0 feature has not been negotiated */
>@@ -408,6 +415,17 @@ static inline bool virtio_is_big_endian(VirtIODevice
>*vdev)
> return false;
> }
>
>+
This adds an extra empty line.
>+/**
>+ * virtio_device_should_start() - check if device started
s/virtio_device_should_start/virtio_device_started/
Best regards,
Bernhard
>+ * @vdev - the VirtIO device
>+ * @status - the devices status bits
>+ *
>+ * Check if the device is started. For most modern machines this is
>+ * tracked via the @vdev->started field (to support migration),
>+ * otherwise we check for the final negotiated status bit that
>+ * indicates everything is ready.
>+ */
> static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status)
> {
> if (vdev->use_started) {
>@@ -428,15 +446,11 @@ static inline bool virtio_device_started(VirtIODevice
>*vdev, uint8_t status)
> */
> static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t
> status)
> {
>-if (vdev->use_started) {
>-return vdev->started;
>-}
>-
> if (!vdev->vm_running) {
> return false;
> }
>
>-return status & VIRTIO_CONFIG_S_DRIVER_OK;
>+return virtio_device_started(vdev, status);
> }
>
> static inline void virtio_set_started(VirtIODevice *vdev, bool started)
