Re: [PATCH for-8.2 0/4] rtc devices: Avoid putting time_t in 32-bit variables
Peter Maydell writes: > On Fri, 21 Jul 2023 at 10:03, Philippe Mathieu-Daudé > wrote: >> >> +Markus >> >> On 20/7/23 17:58, Peter Maydell wrote: >> > This patchset was prompted by a couple of Coverity warnings >> > (CID 1507157, 1517772) which note that in the m48t59 RTC device model >> > we keep an offset in a time_t variable but then truncate it by >> > passing it to qemu_get_timedate(), which currently uses an 'int' >> > argument for its offset parameter. >> > >> > We can fix the Coverity complaint by making qemu_get_timedate() >> > take a time_t; we should also correspondingly make the >> > qemu_timedate_diff() function return a time_t. However this >> > will only push the issue out to callers of qemu_timedate_diff() >> > if they are putting the result in a 32-bit variable or doing >> > 32-bit arithmetic on it. >> > >> > Luckily there aren't that many callers of qemu_timedate_diff() >> > and most of them already use either time_t or int64_t for the >> > calculations they do on its return value. The first three >> > patches fix devices which weren't doing that; patch four then >> > fixes the rtc.c functions. If I missed any callsites in devices >> > then hopefully Coverity will point them out. >> >> Do we need to change the type of the RTC_CHANGE event? [...] > If I understand the 'Built-in Types' section in > docs/devel/qapi-code-gen.rst correctly, the QAPI 'int' > type is already 64 bits. Correct. Also needlessly confusing.
Re: [PULL 06/19] ui/gtk: set scanout-mode right before scheduling draw
Hi there, I guess removing this line would have been causing the problem. Can you add this line back and test it? diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c index eee821d73a..98b3a116bf 100644 --- a/ui/gtk-egl.c +++ b/ui/gtk-egl.c @@ -242,7 +242,6 @@ void gd_egl_scanout_texture(DisplayChangeListener *dcl, eglMakeCurrent(qemu_egl_display, vc->gfx.esurface, vc->gfx.esurface, vc->gfx.ectx); - gtk_egl_set_scanout_mode(vc, true); egl_fb_setup_for_tex(>gfx.guest_fb, backing_width, backing_height, backing_id, false); } Thanks! On 7/20/2023 11:53 PM, Volker Rümelin wrote: Am 17.07.23 um 14:45 schrieb marcandre.lur...@redhat.com: From: Dongwon Kim Setting scanout mode is better to be done very last minute right because the mode can be reset anytime after it is set in dpy_gl_scanout_texture by any asynchronouse dpy_refresh call, which eventually cancels drawing of the guest scanout texture. Hi Dongwon, this patch breaks the QEMU guest display on my system. QEMU was started with ./qemu-system-x86_64 -machine q35 -device virtio-vga-gl,xres=1280,yres=768 -display gtk,zoom-to-fit=off,gl=on. I can see the OVMF boot screen and then GRUB. After Linux was started, plymouth normally shows the OVMF boot logo and a rotating spinner. With your patch the guest screen stays black and I see a text cursor in the upper left corner. It seems the guest works without issues. I can use ssh to log in and I can't find any obvious errors in the guest log files. I tested on a host GNOME desktop under X11 and again under Wayland. In both cases the result is a black guest screen. With best regards, Volker Cc: Gerd Hoffmann Cc: Marc-André Lureau Cc: Vivek Kasireddy Signed-off-by: Dongwon Kim Acked-by: Marc-André Lureau Message-ID:<20230706183355.29361-1-dongwon@intel.com> --- ui/gtk-egl.c | 2 +- ui/gtk-gl-area.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c index eee821d73a..98b3a116bf 100644 --- a/ui/gtk-egl.c +++ b/ui/gtk-egl.c @@ -242,7 +242,6 @@ void gd_egl_scanout_texture(DisplayChangeListener *dcl, eglMakeCurrent(qemu_egl_display, vc->gfx.esurface, vc->gfx.esurface, vc->gfx.ectx); - gtk_egl_set_scanout_mode(vc, true); egl_fb_setup_for_tex(>gfx.guest_fb, backing_width, backing_height, backing_id, false); } @@ -353,6 +352,7 @@ void gd_egl_flush(DisplayChangeListener *dcl, if (vc->gfx.guest_fb.dmabuf && !vc->gfx.guest_fb.dmabuf->draw_submitted) { graphic_hw_gl_block(vc->gfx.dcl.con, true); vc->gfx.guest_fb.dmabuf->draw_submitted = true; + gtk_egl_set_scanout_mode(vc, true); gtk_widget_queue_draw_area(area, x, y, w, h); return; } diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index 4513d3d059..28d9e49888 100644 --- a/ui/gtk-gl-area.c +++ b/ui/gtk-gl-area.c @@ -264,7 +264,6 @@ void gd_gl_area_scanout_texture(DisplayChangeListener *dcl, return; } - gtk_gl_area_set_scanout_mode(vc, true); egl_fb_setup_for_tex(>gfx.guest_fb, backing_width, backing_height, backing_id, false); } @@ -284,6 +283,7 @@ void gd_gl_area_scanout_flush(DisplayChangeListener *dcl, if (vc->gfx.guest_fb.dmabuf && !vc->gfx.guest_fb.dmabuf->draw_submitted) { graphic_hw_gl_block(vc->gfx.dcl.con, true); vc->gfx.guest_fb.dmabuf->draw_submitted = true; + gtk_gl_area_set_scanout_mode(vc, true); } gtk_gl_area_queue_render(GTK_GL_AREA(vc->gfx.drawing_area)); }
Re: [PATCH] target/riscv: Clearing the CSR values at reset and syncing the MPSTATE with the host
On Tue, Jul 18, 2023 at 10:22 PM liguang.zhang <18622748...@163.com> wrote: > > From: "liguang.zhang" > > Fix the guest reboot error when using KVM > There are two issues when rebooting a guest using KVM > 1. When the guest initiates a reboot the host is unable to stop the vcpu > 2. When running a SMP guest the qemu monitor system_reset causes a vcpu crash > > This can be fixed by clearing the CSR values at reset and syncing the > MPSTATE with the host. > > Signed-off-by: liguang.zhang Thanks! When sending new versions of patches please increment the patch version: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#when-resending-patches-add-a-version-tag The patch looks good, but don't we need an equivalent for the get register call? Alistair > --- > target/riscv/kvm.c | 42 > target/riscv/kvm_riscv.h | 1 + > 2 files changed, 43 insertions(+) > > diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c > index 9d8a8982f9..ecc8ab8238 100644 > --- a/target/riscv/kvm.c > +++ b/target/riscv/kvm.c > @@ -44,6 +44,8 @@ > #include "migration/migration.h" > #include "sysemu/runstate.h" > > +static bool cap_has_mp_state; > + > static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type, > uint64_t idx) > { > @@ -790,6 +792,24 @@ int kvm_arch_get_registers(CPUState *cs) > return ret; > } > > +int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state) > +{ > +if (cap_has_mp_state) { > +struct kvm_mp_state mp_state = { > +.mp_state = state > +}; > + > +int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, _state); > +if (ret) { > +fprintf(stderr, "%s: failed to sync MP_STATE %d/%s\n", > +__func__, ret, strerror(-ret)); > +return -1; > +} > +} > + > +return 0; > +} > + > int kvm_arch_put_registers(CPUState *cs, int level) > { > int ret = 0; > @@ -809,6 +829,18 @@ int kvm_arch_put_registers(CPUState *cs, int level) > return ret; > } > > +if (KVM_PUT_RESET_STATE == level) { > +RISCVCPU *cpu = RISCV_CPU(cs); > +if (cs->cpu_index == 0) { > +ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_RUNNABLE); > +} else { > +ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_STOPPED); > +} > +if (ret) { > +return ret; > +} > +} > + > return ret; > } > > @@ -909,6 +941,7 @@ int kvm_arch_add_msi_route_post(struct > kvm_irq_routing_entry *route, > > int kvm_arch_init(MachineState *ms, KVMState *s) > { > +cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE); > return 0; > } > > @@ -987,10 +1020,19 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu) > if (!kvm_enabled()) { > return; > } > +for (int i=0; i<32; i++) > +env->gpr[i] = 0; > env->pc = cpu->env.kernel_addr; > env->gpr[10] = kvm_arch_vcpu_id(CPU(cpu)); /* a0 */ > env->gpr[11] = cpu->env.fdt_addr; /* a1 */ > env->satp = 0; > +env->mie = 0; > +env->stvec = 0; > +env->sscratch = 0; > +env->sepc = 0; > +env->scause = 0; > +env->stval = 0; > +env->mip = 0; > } > > void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level) > diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h > index e3ba935808..3ea68c38e3 100644 > --- a/target/riscv/kvm_riscv.h > +++ b/target/riscv/kvm_riscv.h > @@ -22,5 +22,6 @@ > void kvm_riscv_init_user_properties(Object *cpu_obj); > void kvm_riscv_reset_vcpu(RISCVCPU *cpu); > void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level); > +int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state); > > #endif > -- > 2.17.1 >
Re: [PATCH for-8.2 v5 01/11] target/riscv/cpu.c: split CPU options from riscv_cpu_extensions[]
On Fri, Jul 21, 2023 at 3:20 AM Daniel Henrique Barboza wrote: > > We'll add a new CPU type that will enable a considerable amount of > extensions. To make it easier for us we'll do a few cleanups in our > existing riscv_cpu_extensions[] array. > > Start by splitting all CPU non-boolean options from it. Create a new > riscv_cpu_options[] array for them. Add all these properties in > riscv_cpu_add_user_properties() as it is already being done today. > > 'mmu' and 'pmp' aren't really extensions in the usual way we think about > RISC-V extensions. These are closer to CPU features/options, so move > both to riscv_cpu_options[] too. In the near future we'll need to match > all extensions with all entries in isa_edata_arr[], and so it happens > that both 'mmu' and 'pmp' do not have a riscv,isa string (thus, no priv > spec version restriction). This further emphasizes the point that these > are more a CPU option than an extension. > > No functional changes made. > > Signed-off-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Alistair > --- > target/riscv/cpu.c | 33 +++-- > 1 file changed, 23 insertions(+), 10 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 6b93b04453..9a3afc0482 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1752,7 +1752,6 @@ static void riscv_cpu_add_misa_properties(Object > *cpu_obj) > > static Property riscv_cpu_extensions[] = { > /* Defaults for standard extensions */ > -DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16), > DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false), > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true), > @@ -1764,15 +1763,8 @@ static Property riscv_cpu_extensions[] = { > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false), > DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, false), > DEFINE_PROP_BOOL("Zve64d", RISCVCPU, cfg.ext_zve64d, false), > -DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true), > -DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true), > DEFINE_PROP_BOOL("sstc", RISCVCPU, cfg.ext_sstc, true), > > -DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec), > -DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec), > -DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128), > -DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64), > - > DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen, false), > DEFINE_PROP_BOOL("svadu", RISCVCPU, cfg.ext_svadu, true), > DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false), > @@ -1803,9 +1795,7 @@ static Property riscv_cpu_extensions[] = { > DEFINE_PROP_BOOL("zhinxmin", RISCVCPU, cfg.ext_zhinxmin, false), > > DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true), > -DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64), > DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true), > -DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64), > > DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false), > > @@ -1849,6 +1839,21 @@ static Property riscv_cpu_extensions[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > +static Property riscv_cpu_options[] = { > +DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16), > + > +DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true), > +DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true), > + > +DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec), > +DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec), > + > +DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128), > +DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64), > + > +DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64), > +DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64), > +}; > > #ifndef CONFIG_USER_ONLY > static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, > @@ -1917,6 +1922,14 @@ static void riscv_cpu_add_user_properties(Object *obj) > #endif > qdev_property_add_static(dev, prop); > } > + > +for (int i = 0; i < ARRAY_SIZE(riscv_cpu_options); i++) { > +/* Check if KVM created the property already */ > +if (object_property_find(obj, riscv_cpu_options[i].name)) { > +continue; > +} > +qdev_property_add_static(dev, _cpu_options[i]); > +} > } > > static Property riscv_cpu_properties[] = { > -- > 2.41.0 > >
Re: [PATCH 1/1] linux-user: add support for big endian variants of riscv
On Fri, Jun 30, 2023 at 7:45 AM Palmer Dabbelt wrote: > > On Fri, 30 Jun 2023 04:14:09 PDT (-0700), rory.opensou...@gmail.com wrote: > > RISCV architecture supports an optional big endian mode of operation. > > In this mode, data accesses are treated as big endian, while code is > > always in little endian format. This is similar to how the ARM > > architecture treats it's optional bi-endian support. This patch adds > > support for big endian RISCV operation to linux-user. > > We don't have BE support in Linux yet. IIRC we've had some other > linux-user stuff go in with a "we'll change it to match whatever uABI > Linux ends up with" sort of caveat, but I might be mistaken. I'm not > opposed to doing that sort of thing for BE as well. I don't remember > what the right way to indicate that is, though. > > > Signed-off-by: rory.opensou...@gmail.com > > --- > > configs/targets/riscv64be-linux-user.mak| 7 +++ > > configure | 1 + > > linux-user/elfload.c| 10 ++ > > linux-user/include/host/riscv/host-signal.h | 3 +++ > > linux-user/riscv/signal.c | 5 + > > linux-user/riscv/target_syscall.h | 8 > > scripts/probe-gdb-support.py| 4 ++-- > > scripts/qemu-binfmt-conf.sh | 12 ++-- > > target/riscv/cpu.c | 5 + > > target/riscv/translate.c| 13 + > > 10 files changed, 64 insertions(+), 4 deletions(-) > > create mode 100644 configs/targets/riscv64be-linux-user.mak > > > > diff --git a/configs/targets/riscv64be-linux-user.mak > > b/configs/targets/riscv64be-linux-user.mak > > new file mode 100644 > > index 00..f22f5f0971 > > --- /dev/null > > +++ b/configs/targets/riscv64be-linux-user.mak > > @@ -0,0 +1,7 @@ > > +TARGET_ARCH=riscv64 > > +TARGET_BASE_ARCH=riscv > > +TARGET_ABI_DIR=riscv > > +TARGET_BIG_ENDIAN=y > > +TARGET_XML_FILES= gdb-xml/riscv-64bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml > > gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-64bit-virtual.xml > > +CONFIG_SEMIHOSTING=y > > +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y > > diff --git a/configure b/configure > > index 2b41c49c0d..90795a0e9f 100755 > > --- a/configure > > +++ b/configure > > @@ -1190,6 +1190,7 @@ fi > > : ${cross_prefix_ppc64="powerpc64-linux-gnu-"} > > : ${cross_prefix_ppc64le="$cross_prefix_ppc64"} > > : ${cross_prefix_riscv64="riscv64-linux-gnu-"} > > +: ${cross_prefix_riscv64be="riscv64be-linux-gnu-"} > > : ${cross_prefix_s390x="s390x-linux-gnu-"} > > : ${cross_prefix_sh4="sh4-linux-gnu-"} > > : ${cross_prefix_sparc64="sparc64-linux-gnu-"} > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c > > index 9a2ec568b0..e0204c7069 100644 > > --- a/linux-user/elfload.c > > +++ b/linux-user/elfload.c > > @@ -1681,8 +1681,18 @@ static void elf_core_copy_regs(target_elf_gregset_t > > *regs, > > > > #ifdef TARGET_RISCV32 > > #define ELF_CLASS ELFCLASS32 > > +#if TARGET_BIG_ENDIAN > > +#define ELF_PLATFORM "riscv32be" > > +#else > > +#define ELF_PLATFORM "riscv32" > > +#endif > > #else > > #define ELF_CLASS ELFCLASS64 > > +#if TARGET_BIG_ENDIAN > > +#define ELF_PLATFORM "riscv64be" > > +#else > > +#define ELF_PLATFORM "riscv64" > > +#endif > > #endif > > > > #define ELF_HWCAP get_elf_hwcap() > > diff --git a/linux-user/include/host/riscv/host-signal.h > > b/linux-user/include/host/riscv/host-signal.h > > index decacb2325..b3f2735261 100644 > > --- a/linux-user/include/host/riscv/host-signal.h > > +++ b/linux-user/include/host/riscv/host-signal.h > > @@ -38,6 +38,9 @@ static inline bool host_signal_write(siginfo_t *info, > > host_sigcontext *uc) > > */ > > const uint16_t *pinsn = (const uint16_t *)host_signal_pc(uc); > > uint16_t insn = pinsn[0]; > > +#if TARGET_BIG_ENDIAN > > +insn = (insn << 8) | (insn >> 8); > > +#endif > > > > /* 16-bit instructions */ > > switch (insn & 0xe003) { > > diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c > > index eaa168199a..1d9e3413fb 100644 > > --- a/linux-user/riscv/signal.c > > +++ b/linux-user/riscv/signal.c > > @@ -199,8 +199,13 @@ void setup_sigtramp(abi_ulong sigtramp_page) > > uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 8, 0); > > assert(tramp != NULL); > > > > +#if TARGET_BIG_ENDIAN > > +__put_user(0x9308b008, tramp + 0); /* li a7, 139 = __NR_rt_sigreturn > > */ > > +__put_user(0x7300, tramp + 1); /* ecall */ > > +#else > > __put_user(0x08b00893, tramp + 0); /* li a7, 139 = __NR_rt_sigreturn > > */ > > __put_user(0x0073, tramp + 1); /* ecall */ > > +#endif > > > > default_rt_sigreturn = sigtramp_page; > > unlock_user(tramp, sigtramp_page, 8); > > diff --git a/linux-user/riscv/target_syscall.h > > b/linux-user/riscv/target_syscall.h > > index 7601f10c28..88c0ac1351 100644 > > --- a/linux-user/riscv/target_syscall.h > > +++
Re: [PATCH 0/2] target/riscv: add missing riscv,isa strings
On Thu, Jul 20, 2023 at 11:25 PM Daniel Henrique Barboza wrote: > > Hi, > > Found these 2 instances while working in more 8.2 material. > > I believe both are safe for freeze but I won't lose my sleep if we > decide to postpone it. I wasn't going to squeeze them into the freeze > > Daniel Henrique Barboza (2): > target/riscv/cpu.c: add zmmul isa string > target/riscv/cpu.c: add smepmp isa string Do you mind rebasing :) https://github.com/alistair23/qemu/tree/riscv-to-apply.next Alistair > > target/riscv/cpu.c | 2 ++ > 1 file changed, 2 insertions(+) > > -- > 2.41.0 > >
Re: [PATCH 2/2] target/riscv/cpu.c: add smepmp isa string
On Thu, Jul 20, 2023 at 11:26 PM Daniel Henrique Barboza wrote: > > The cpu->cfg.epmp extension is still experimental, but it already has a > 'smepmp' riscv,isa string. Add it. > > Signed-off-by: Daniel Henrique Barboza Thanks! Applied to riscv-to-apply.next Alistair > --- > target/riscv/cpu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index d64ac07558..8c9acadd3b 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -130,6 +130,7 @@ static const struct isa_ext_data isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx), > ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), > ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia), > +ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, epmp), > ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen), > ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf), > -- > 2.41.0 > >
Re: [PATCH 5/5] hw/riscv: virt: support for RISC-V IOMMU platform device.
On Thu, Jul 20, 2023 at 12:35 PM Tomasz Jeznach wrote: > > Adding virt machine property 'iommu' to enable/disable IOMMU > support, with platform RISC-V IOMMU device implementation. > > Generate device tree entry for riscv-iommu device, along with > mapping all PCI device identifiers to the single IOMMU device > instance. > > Signed-off-by: Tomasz Jeznach > --- > hw/riscv/Kconfig| 1 + > hw/riscv/virt.c | 100 +++- > include/hw/riscv/virt.h | 3 ++ > 3 files changed, 103 insertions(+), 1 deletion(-) > > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig > index 617a509f1b..b1a3a9994f 100644 > --- a/hw/riscv/Kconfig > +++ b/hw/riscv/Kconfig > @@ -41,6 +41,7 @@ config RISCV_VIRT > select SERIAL > select RISCV_ACLINT > select RISCV_APLIC > +select RISCV_IOMMU > select RISCV_IMSIC > select SIFIVE_PLIC > select SIFIVE_TEST > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index d90286dc46..49cc7105af 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -32,6 +32,7 @@ > #include "hw/core/sysbus-fdt.h" > #include "target/riscv/pmu.h" > #include "hw/riscv/riscv_hart.h" > +#include "hw/riscv/iommu.h" > #include "hw/riscv/virt.h" > #include "hw/riscv/boot.h" > #include "hw/riscv/numa.h" > @@ -88,7 +89,8 @@ static const MemMapEntry virt_memmap[] = { > [VIRT_APLIC_M] = { 0xc00, APLIC_SIZE(VIRT_CPUS_MAX) }, > [VIRT_APLIC_S] = { 0xd00, APLIC_SIZE(VIRT_CPUS_MAX) }, > [VIRT_UART0] ={ 0x1000, 0x100 }, > -[VIRT_VIRTIO] = { 0x10001000,0x1000 }, > +[VIRT_IOMMU] ={ 0x10001000,0x1000 }, > +[VIRT_VIRTIO] = { 0x10008000,0x1000 }, /* VIRTIO_COUNT */ We shouldn't change existing addresses Alistair > [VIRT_FW_CFG] = { 0x1010, 0x18 }, > [VIRT_FLASH] ={ 0x2000, 0x400 }, > [VIRT_IMSIC_M] = { 0x2400, VIRT_IMSIC_MAX_SIZE }, > @@ -1019,6 +1021,44 @@ static void create_fdt_fw_cfg(RISCVVirtState *s, const > MemMapEntry *memmap) > g_free(nodename); > } > > +static void create_fdt_iommu(RISCVVirtState *s, const MemMapEntry *memmap, > +uint32_t irq_mmio_phandle) > +{ > +MachineState *ms = MACHINE(s); > +uint32_t iommu_phandle; > +const char *irq_names[] = { "cmdq", "fltq", "pm", "priq" }; > +char *iommu_node; > +char *pci_node; > + > +pci_node = g_strdup_printf("/soc/pci@%" PRIx64, > memmap[VIRT_PCIE_ECAM].base); > +iommu_node = g_strdup_printf("/soc/iommu@%" PRIx64, > memmap[VIRT_IOMMU].base); > + > +iommu_phandle = qemu_fdt_alloc_phandle(ms->fdt); > +qemu_fdt_add_subnode(ms->fdt, iommu_node); > +qemu_fdt_setprop_string(ms->fdt, iommu_node, "compatible", > "riscv,iommu"); > +qemu_fdt_setprop_cell(ms->fdt, iommu_node, "#iommu-cells", 1); > +qemu_fdt_setprop_cell(ms->fdt, iommu_node, "phandle", iommu_phandle); > +qemu_fdt_setprop_cells(ms->fdt, iommu_node, "reg", > +0x0, memmap[VIRT_IOMMU].base, 0x0, memmap[VIRT_IOMMU].size); > +qemu_fdt_setprop_cell(ms->fdt, iommu_node, "interrupt-parent", > irq_mmio_phandle); > +qemu_fdt_setprop_string_array(ms->fdt, iommu_node, "interrupt-names", > +(char **) _names, ARRAY_SIZE(irq_names)); > +qemu_fdt_setprop_cells(ms->fdt, iommu_node, "interrupts", > +IOMMU_IRQ + 0, 0x4, > +IOMMU_IRQ + 1, 0x4, > +IOMMU_IRQ + 2, 0x4, > +IOMMU_IRQ + 3, 0x4); > +qemu_fdt_setprop_cells(ms->fdt, pci_node, "iommu-map", > +0x0, iommu_phandle, 0x0, 0x); > +g_free(iommu_node); > +g_free(pci_node); > +} > + > +static bool virt_is_iommu_enabled(RISCVVirtState *s) > +{ > +return s->iommu != ON_OFF_AUTO_OFF; > +} > + > static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap) > { > MachineState *ms = MACHINE(s); > @@ -1051,6 +1091,10 @@ static void create_fdt(RISCVVirtState *s, const > MemMapEntry *memmap) > > create_fdt_pcie(s, memmap, irq_pcie_phandle, msi_pcie_phandle); > > +if (virt_is_iommu_enabled(s)) { > +create_fdt_iommu(s, memmap, irq_mmio_phandle); > +} > + > create_fdt_reset(s, memmap, ); > > create_fdt_uart(s, memmap, irq_mmio_phandle); > @@ -1210,6 +1254,31 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType > aia_type, int aia_guests, > return aplic_m; > } > > +static DeviceState *virt_create_iommu(RISCVVirtState *s, DeviceState > *irqchip) > +{ > +DeviceState *iommu; > +int i; > + > +iommu = qdev_new(TYPE_RISCV_IOMMU_SYS); > + > +if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) { > +/* Disable MSI_FLAT [22], MSI_MRIF [23] if IMSIC is not enabled. */ > +qdev_prop_set_uint64(iommu, "capabilities", ~(BIT_ULL(22) | > BIT_ULL(23))); > +} > + > +/* Fixed base register address */ > +qdev_prop_set_uint64(iommu, "addr", virt_memmap[VIRT_IOMMU].base); > + > +
Re: [PATCH for-8.2 2/2] arm/kvm: convert to kvm_get_one_reg
Hi Connie, On 7/18/23 21:14, Cornelia Huck wrote: We can neaten the code by switching the callers that work on a CPUstate to the kvm_get_one_reg function. Signed-off-by: Cornelia Huck --- target/arm/kvm.c | 15 +++- target/arm/kvm64.c | 57 -- 2 files changed, 18 insertions(+), 54 deletions(-) The replacements look good to me. However, I guess it's worty to apply the same replacements for target/arm/kvm64.c since we're here? [gshan@gshan arm]$ pwd /home/gshan/sandbox/q/target/arm [gshan@gshan arm]$ git grep KVM_GET_ONE_REG kvm64.c:err = ioctl(fd, KVM_GET_ONE_REG, ); kvm64.c:return ioctl(fd, KVM_GET_ONE_REG, ); kvm64.c:ret = ioctl(fdarray[2], KVM_GET_ONE_REG, ); Thanks, Gavin diff --git a/target/arm/kvm.c b/target/arm/kvm.c index cdbffc3c6e0d..4123f6dc9d72 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -525,24 +525,19 @@ bool write_kvmstate_to_list(ARMCPU *cpu) bool ok = true; for (i = 0; i < cpu->cpreg_array_len; i++) { -struct kvm_one_reg r; uint64_t regidx = cpu->cpreg_indexes[i]; uint32_t v32; int ret; -r.id = regidx; - switch (regidx & KVM_REG_SIZE_MASK) { case KVM_REG_SIZE_U32: -r.addr = (uintptr_t) -ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +ret = kvm_get_one_reg(cs, regidx, ); if (!ret) { cpu->cpreg_values[i] = v32; } break; case KVM_REG_SIZE_U64: -r.addr = (uintptr_t)(cpu->cpreg_values + i); -ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +ret = kvm_get_one_reg(cs, regidx, cpu->cpreg_values + i); break; default: g_assert_not_reached(); @@ -678,17 +673,13 @@ int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu) void kvm_arm_get_virtual_time(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); -struct kvm_one_reg reg = { -.id = KVM_REG_ARM_TIMER_CNT, -.addr = (uintptr_t)>kvm_vtime, -}; int ret; if (cpu->kvm_vtime_dirty) { return; } -ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +ret = kvm_get_one_reg(cs, KVM_REG_ARM_TIMER_CNT, >kvm_vtime); if (ret) { error_report("Failed to get KVM_REG_ARM_TIMER_CNT"); abort(); diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index b4d02dff5381..66b52d6f8d23 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -908,14 +908,11 @@ int kvm_arch_put_registers(CPUState *cs, int level) static int kvm_arch_get_fpsimd(CPUState *cs) { CPUARMState *env = _CPU(cs)->env; -struct kvm_one_reg reg; int i, ret; for (i = 0; i < 32; i++) { uint64_t *q = aa64_vfp_qreg(env, i); -reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); -reg.addr = (uintptr_t)q; -ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +ret = kvm_get_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), q); if (ret) { return ret; } else { @@ -939,15 +936,12 @@ static int kvm_arch_get_sve(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); CPUARMState *env = >env; -struct kvm_one_reg reg; uint64_t *r; int n, ret; for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) { r = >vfp.zregs[n].d[0]; -reg.addr = (uintptr_t)r; -reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0); -ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_ZREG(n, 0), r); if (ret) { return ret; } @@ -956,9 +950,7 @@ static int kvm_arch_get_sve(CPUState *cs) for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) { r = >vfp.pregs[n].p[0]; -reg.addr = (uintptr_t)r; -reg.id = KVM_REG_ARM64_SVE_PREG(n, 0); -ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_PREG(n, 0), r); if (ret) { return ret; } @@ -966,9 +958,7 @@ static int kvm_arch_get_sve(CPUState *cs) } r = >vfp.pregs[FFR_PRED_NUM].p[0]; -reg.addr = (uintptr_t)r; -reg.id = KVM_REG_ARM64_SVE_FFR(0); -ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_FFR(0), r); if (ret) { return ret; } @@ -979,7 +969,6 @@ static int kvm_arch_get_sve(CPUState *cs) int kvm_arch_get_registers(CPUState *cs) { -struct kvm_one_reg reg; uint64_t val; unsigned int el; uint32_t fpr; @@ -989,31 +978,24 @@ int kvm_arch_get_registers(CPUState *cs) CPUARMState *env = >env; for (i = 0; i < 31; i++) { -reg.id = AARCH64_CORE_REG(regs.regs[i]); -reg.addr = (uintptr_t) >xregs[i]; -ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ); +ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.regs[i]), +
Re: [RFC 0/1] virtio-net: add support for SR-IOV emulation
On 2023/07/20 11:20, Jason Wang wrote: On Wed, Jul 19, 2023 at 9:59 AM Yui Washizu wrote: This patch series is the first step towards enabling hardware offloading of the L2 packet switching feature on virtio-net device to host machine. We are considering that this hardware offloading enables the use of high-performance networks in virtual infrastructures, such as container infrastructures on VMs. To enable L2 packet switching by SR-IOV VFs, we are considering the following: - making the guest recognize virtio-net devices as SR-IOV PF devices (archived with this patch series) - allowing virtio-net devices to connect SR-IOV VFs to the backend networks, leaving the L2 packet switching feature to the management layer like libvirt Could you please show the qemu command line you want to propose here? I am considering how to specify the properties of VFs to connect SR-IOV VFs to the backend networks. For example: qemu-system-x86_64 -device pcie-root-port,port=8,chassis=8,id=pci.8,bus=pcie.0,multifunction=on -netdev tap,id=hostnet0,vhost=on -netdev tap,id=vfnet1,vhost=on # backend network for SR-IOV VF 1 -netdev tap,id=vfnet2,vhost=on # backend network for SR-IOV VF 2 -device virtio-net-pci,netdev=hostnet0,sriov_max_vfs=2,sriov_netdev=vfnet1:vfnet2,... In this example, we can specify multiple backend networks to the VFs by adding "sriov_netdev" and separating them with ":". Additionally, when passing properties like "rx_queue_size" to VFs, we can utilize new properties, such as "sriov_rx_queue_size_per_vfs," to ensure that the same value is passed to all VFs. I'm still considering about how to specify it, so please give me any comments if you have any. - This makes hardware offloading of L2 packet switching possible. For example, when using vDPA devices, it allows the guest to utilize SR-IOV NIC embedded switch of hosts. This would be interesting. Thanks This patch series aims to enable SR-IOV emulation on virtio-net devices. With this series, the guest can identify the virtio-net device as an SR-IOV PF device. The newly added property 'sriov_max_vfs' allows us to enable the SR-IOV feature on the virtio-net device. Currently, we are unable to specify the properties of a VF created from the guest. The properties are set to their default values. In the future, we plan to allow users to set the properties. qemu-system-x86_64 --device virtio-net,sriov_max_vfs= # when 'sriov_max_vfs' is present, the SR-IOV feature will be automatically enabled # means the max number of VF on guest Example commands to create VFs in virtio-net device from the guest: guest% readlink -f /sys/class/net/eth1/device /sys/devices/pci:00/:00:02.0/:01:00.0/virtio1 guest% echo "2" > /sys/devices/pci:00/:00:02.0/:01:00.0/sriov_numvfs guest% ip link show eth0: eth1: eth2: #virtual VF created eth3: #virtual VF created Please note that communication between VF and PF/VF is not possible by this patch series itself. Yui Washizu (1): virtio-pci: add SR-IOV capability hw/pci/msix.c | 8 +++-- hw/pci/pci.c | 4 +++ hw/virtio/virtio-pci.c | 62 ++ include/hw/virtio/virtio-pci.h | 1 + 4 files changed, 66 insertions(+), 9 deletions(-) -- 2.39.3
Re: [PATCH for-8.2 1/2] arm/kvm: convert to kvm_set_one_reg
Hi Connie, On 7/18/23 21:14, Cornelia Huck wrote: We can neaten the code by switching to the kvm_set_one_reg function. Signed-off-by: Cornelia Huck --- target/arm/kvm.c | 13 +++-- target/arm/kvm64.c | 66 +- 2 files changed, 21 insertions(+), 58 deletions(-) Some wrong replacements to be fixed in kvm_arch_put_fpsimd() as below. Apart from that, LGTM: Reviewed-by: Gavin Shan diff --git a/target/arm/kvm.c b/target/arm/kvm.c index b4c7654f4980..cdbffc3c6e0d 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -561,7 +561,6 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level) bool ok = true; for (i = 0; i < cpu->cpreg_array_len; i++) { -struct kvm_one_reg r; uint64_t regidx = cpu->cpreg_indexes[i]; uint32_t v32; int ret; @@ -570,19 +569,17 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level) continue; } -r.id = regidx; switch (regidx & KVM_REG_SIZE_MASK) { case KVM_REG_SIZE_U32: v32 = cpu->cpreg_values[i]; -r.addr = (uintptr_t) +ret = kvm_set_one_reg(cs, regidx, ); break; case KVM_REG_SIZE_U64: -r.addr = (uintptr_t)(cpu->cpreg_values + i); +ret = kvm_set_one_reg(cs, regidx, cpu->cpreg_values + i); break; default: g_assert_not_reached(); } -ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ); if (ret) { /* We might fail for "unknown register" and also for * "you tried to set a register which is constant with @@ -703,17 +700,13 @@ void kvm_arm_get_virtual_time(CPUState *cs) void kvm_arm_put_virtual_time(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); -struct kvm_one_reg reg = { -.id = KVM_REG_ARM_TIMER_CNT, -.addr = (uintptr_t)>kvm_vtime, -}; int ret; if (!cpu->kvm_vtime_dirty) { return; } -ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ); +ret = kvm_set_one_reg(cs, KVM_REG_ARM_TIMER_CNT, >kvm_vtime); if (ret) { error_report("Failed to set KVM_REG_ARM_TIMER_CNT"); abort(); diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 94bbd9661fd3..b4d02dff5381 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -540,14 +540,10 @@ static int kvm_arm_sve_set_vls(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); uint64_t vls[KVM_ARM64_SVE_VLS_WORDS] = { cpu->sve_vq.map }; -struct kvm_one_reg reg = { -.id = KVM_REG_ARM64_SVE_VLS, -.addr = (uint64_t)[0], -}; assert(cpu->sve_max_vq <= KVM_ARM64_SVE_VQ_MAX); -return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ); +return kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_VLS, [0]); } #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 @@ -725,19 +721,17 @@ static void kvm_inject_arm_sea(CPUState *c) static int kvm_arch_put_fpsimd(CPUState *cs) { CPUARMState *env = _CPU(cs)->env; -struct kvm_one_reg reg; int i, ret; for (i = 0; i < 32; i++) { uint64_t *q = aa64_vfp_qreg(env, i); #if HOST_BIG_ENDIAN uint64_t fp_val[2] = { q[1], q[0] }; -reg.addr = (uintptr_t)fp_val; +ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), +_val); ^^^ s/_val/fp_val #else -reg.addr = (uintptr_t)q; +ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), ); ^^^ s//q #endif -reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); -ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ); if (ret) { return ret; } @@ -758,14 +752,11 @@ static int kvm_arch_put_sve(CPUState *cs) CPUARMState *env = >env; uint64_t tmp[ARM_MAX_VQ * 2]; uint64_t *r; -struct kvm_one_reg reg; int n, ret; for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) { r = sve_bswap64(tmp, >vfp.zregs[n].d[0], cpu->sve_max_vq * 2); -reg.addr = (uintptr_t)r; -reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0); -ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ); +ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_ZREG(n, 0), r); if (ret) { return ret; } @@ -774,9 +765,7 @@ static int kvm_arch_put_sve(CPUState *cs) for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) { r = sve_bswap64(tmp, r = >vfp.pregs[n].p[0], DIV_ROUND_UP(cpu->sve_max_vq * 2, 8)); -reg.addr = (uintptr_t)r; -
Re: [PATCH for-8.2 2/2] target/riscv/cpu.c: add zihpm extension flag
On Tue, Jul 18, 2023 at 7:55 AM Daniel Henrique Barboza wrote: > > zihpm is the Hardware Performance Counters extension described in > chapter 12 of the unprivileged spec. It describes support for 29 > unprivileged performance counters, hpmcounter3-hpmcounter21. > > As with zicntr, QEMU already implements zihpm before it was even an > extension. zihpm is also part of the RVA22 profile, so add it to QEMU > to complement the future future profile implementation. > > Default it to 'true' since it was always present in the code. Change the > realize() time validation to disable it in case 'icsr' isn't present and > if there's no hardware counters (cpu->cfg.pmu_num is zero). > > There's a small tweak needed in riscv_cpu_realize_tcg() made: > riscv_cpu_validate_set_extensions() must be executed after the block > that executes riscv_pmu_init(). The reason is that riscv_pmu_init() will > do "cpu->cfg.pmu_num = 0" if PMU support cannot be enabled. We want to > get the latest, definite value of cfg.pmu_num during the validation() to > ensure we do the right thing. > > Signed-off-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Alistair > --- > target/riscv/cpu.c | 20 +--- > target/riscv/cpu_cfg.h | 1 + > 2 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 7ec88659be..5836640d5c 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -89,6 +89,7 @@ static const struct isa_ext_data isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), > ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), > ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > +ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_ihpm), > ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs), > ISA_EXT_DATA_ENTRY(zfa, PRIV_VERSION_1_12_0, ext_zfa), > ISA_EXT_DATA_ENTRY(zfbfmin, PRIV_VERSION_1_12_0, ext_zfbfmin), > @@ -1296,6 +1297,10 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, > Error **errp) > cpu->cfg.ext_icntr = false; > } > > +if (cpu->cfg.ext_ihpm && (!cpu->cfg.ext_icsr || cpu->cfg.pmu_num == 0)) { > +cpu->cfg.ext_ihpm = false; > +} > + > /* > * Disable isa extensions based on priv spec after we > * validated and set everything we need. > @@ -1426,12 +1431,6 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, > Error **errp) > return; > } > > -riscv_cpu_validate_set_extensions(cpu, _err); > -if (local_err != NULL) { > -error_propagate(errp, local_err); > -return; > -} > - > #ifndef CONFIG_USER_ONLY > CPU(dev)->tcg_cflags |= CF_PCREL; > > @@ -1446,6 +1445,12 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, > Error **errp) > } > } > #endif > + > +riscv_cpu_validate_set_extensions(cpu, _err); > +if (local_err != NULL) { > +error_propagate(errp, local_err); > +return; > +} > } > > static void riscv_cpu_realize(DeviceState *dev, Error **errp) > @@ -1784,10 +1789,11 @@ static Property riscv_cpu_extensions[] = { > DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false), > > /* > - * Always default true - we'll disable it during > + * Always default true - we'll disable them during > * realize() if needed. > */ > DEFINE_PROP_BOOL("zicntr", RISCVCPU, cfg.ext_icntr, true), > +DEFINE_PROP_BOOL("zihpm", RISCVCPU, cfg.ext_ihpm, true), > > DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true), > DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true), > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h > index d36dc12b92..85c7a71853 100644 > --- a/target/riscv/cpu_cfg.h > +++ b/target/riscv/cpu_cfg.h > @@ -66,6 +66,7 @@ struct RISCVCPUConfig { > bool ext_icsr; > bool ext_icbom; > bool ext_icboz; > +bool ext_ihpm; > bool ext_zicond; > bool ext_zihintpause; > bool ext_smstateen; > -- > 2.41.0 > >
Re: [PATCH for-8.2 1/2] target/riscv/cpu.c: add zicntr extension flag
On Tue, Jul 18, 2023 at 7:55 AM Daniel Henrique Barboza wrote: > > zicntr is the Base Counters and Timers extension described in chapter 12 > of the unprivileged spec. It describes support for RDCYCLE, RDTIME and > RDINSTRET. > > QEMU already implements it way before it was a discrete extension. > zicntr is part of the RVA22 profile, so let's add it to QEMU to make the > future profile implementation flag complete. > > Given than it represents an already existing feature, default it to > 'true'. Change the realize() time validation to disable it in case its > dependency (icsr) isn't present. > > Signed-off-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Alistair > --- > target/riscv/cpu.c | 11 +++ > target/riscv/cpu_cfg.h | 1 + > 2 files changed, 12 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 9339c0241d..7ec88659be 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -85,6 +85,7 @@ static const struct isa_ext_data isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_icbom), > ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_icboz), > ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond), > +ISA_EXT_DATA_ENTRY(zicntr, PRIV_VERSION_1_12_0, ext_icntr), > ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), > ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), > ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > @@ -1291,6 +1292,10 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, > Error **errp) > cpu->cfg.ext_zksh = true; > } > > +if (cpu->cfg.ext_icntr && !cpu->cfg.ext_icsr) { > +cpu->cfg.ext_icntr = false; > +} > + > /* > * Disable isa extensions based on priv spec after we > * validated and set everything we need. > @@ -1778,6 +1783,12 @@ static Property riscv_cpu_extensions[] = { > DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false), > DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false), > > +/* > + * Always default true - we'll disable it during > + * realize() if needed. > + */ > +DEFINE_PROP_BOOL("zicntr", RISCVCPU, cfg.ext_icntr, true), > + > DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true), > DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true), > DEFINE_PROP_BOOL("zbc", RISCVCPU, cfg.ext_zbc, true), > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h > index 2bd9510ba3..d36dc12b92 100644 > --- a/target/riscv/cpu_cfg.h > +++ b/target/riscv/cpu_cfg.h > @@ -62,6 +62,7 @@ struct RISCVCPUConfig { > bool ext_zksh; > bool ext_zkt; > bool ext_ifencei; > +bool ext_icntr; > bool ext_icsr; > bool ext_icbom; > bool ext_icboz; > -- > 2.41.0 > >
Re: [PATCH 03/10] hw/riscv: virt: Make few IMSIC macros and functions public
On Thu, Jul 13, 2023 at 2:42 AM Sunil V L wrote: > > Some macros and static function related to IMSIC are defined > in virt.c. They are required in virt-acpi-build.c. So, make them > public. > > Signed-off-by: Sunil V L Reviewed-by: Alistair Francis Alistair > --- > hw/riscv/virt.c | 25 + > include/hw/riscv/virt.h | 25 + > 2 files changed, 26 insertions(+), 24 deletions(-) > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index 46d3341113..f6067db8ec 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -37,7 +37,6 @@ > #include "hw/riscv/numa.h" > #include "hw/intc/riscv_aclint.h" > #include "hw/intc/riscv_aplic.h" > -#include "hw/intc/riscv_imsic.h" > #include "hw/intc/sifive_plic.h" > #include "hw/misc/sifive_test.h" > #include "hw/platform-bus.h" > @@ -53,28 +52,6 @@ > #include "hw/acpi/aml-build.h" > #include "qapi/qapi-visit-common.h" > > -/* > - * The virt machine physical address space used by some of the devices > - * namely ACLINT, PLIC, APLIC, and IMSIC depend on number of Sockets, > - * number of CPUs, and number of IMSIC guest files. > - * > - * Various limits defined by VIRT_SOCKETS_MAX_BITS, VIRT_CPUS_MAX_BITS, > - * and VIRT_IRQCHIP_MAX_GUESTS_BITS are tuned for maximum utilization > - * of virt machine physical address space. > - */ > - > -#define VIRT_IMSIC_GROUP_MAX_SIZE (1U << IMSIC_MMIO_GROUP_MIN_SHIFT) > -#if VIRT_IMSIC_GROUP_MAX_SIZE < \ > -IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS) > -#error "Can't accomodate single IMSIC group in address space" > -#endif > - > -#define VIRT_IMSIC_MAX_SIZE(VIRT_SOCKETS_MAX * \ > -VIRT_IMSIC_GROUP_MAX_SIZE) > -#if 0x400 < VIRT_IMSIC_MAX_SIZE > -#error "Can't accomodate all IMSIC groups in address space" > -#endif > - > static const MemMapEntry virt_memmap[] = { > [VIRT_DEBUG] ={0x0, 0x100 }, > [VIRT_MROM] = { 0x1000,0xf000 }, > @@ -505,7 +482,7 @@ static void create_fdt_socket_plic(RISCVVirtState *s, > g_free(plic_cells); > } > > -static uint32_t imsic_num_bits(uint32_t count) > +uint32_t imsic_num_bits(uint32_t count) > { > uint32_t ret = 0; > > diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h > index 4ef1f660ab..00c22492a7 100644 > --- a/include/hw/riscv/virt.h > +++ b/include/hw/riscv/virt.h > @@ -23,6 +23,7 @@ > #include "hw/riscv/riscv_hart.h" > #include "hw/sysbus.h" > #include "hw/block/flash.h" > +#include "hw/intc/riscv_imsic.h" > > #define VIRT_CPUS_MAX_BITS 9 > #define VIRT_CPUS_MAX (1 << VIRT_CPUS_MAX_BITS) > @@ -128,4 +129,28 @@ enum { > > bool virt_is_acpi_enabled(RISCVVirtState *s); > void virt_acpi_setup(RISCVVirtState *vms); > +uint32_t imsic_num_bits(uint32_t count); > + > +/* > + * The virt machine physical address space used by some of the devices > + * namely ACLINT, PLIC, APLIC, and IMSIC depend on number of Sockets, > + * number of CPUs, and number of IMSIC guest files. > + * > + * Various limits defined by VIRT_SOCKETS_MAX_BITS, VIRT_CPUS_MAX_BITS, > + * and VIRT_IRQCHIP_MAX_GUESTS_BITS are tuned for maximum utilization > + * of virt machine physical address space. > + */ > + > +#define VIRT_IMSIC_GROUP_MAX_SIZE (1U << IMSIC_MMIO_GROUP_MIN_SHIFT) > +#if VIRT_IMSIC_GROUP_MAX_SIZE < \ > +IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS) > +#error "Can't accomodate single IMSIC group in address space" > +#endif > + > +#define VIRT_IMSIC_MAX_SIZE(VIRT_SOCKETS_MAX * \ > +VIRT_IMSIC_GROUP_MAX_SIZE) > +#if 0x400 < VIRT_IMSIC_MAX_SIZE > +#error "Can't accomodate all IMSIC groups in address space" > +#endif > + > #endif > -- > 2.39.2 > >
Re: [PATCH 02/10] hw/riscv: virt: Add PCI bus reference in RISCVVirtState
On Thu, Jul 13, 2023 at 2:42 AM Sunil V L wrote: > > The PCI bus information is needed in RISCVVirtState so that other > files like virt-acpi-build.c can make use of it. Add new field in > RISCVVirtState so that ACPI code can use it. > > Signed-off-by: Sunil V L Reviewed-by: Alistair Francis Alistair > --- > hw/riscv/virt.c | 6 -- > include/hw/riscv/virt.h | 1 + > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index d90286dc46..46d3341113 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -1073,7 +1073,8 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion > *sys_mem, >hwaddr high_mmio_base, >hwaddr high_mmio_size, >hwaddr pio_base, > - DeviceState *irqchip) > + DeviceState *irqchip, > + RISCVVirtState *s) > { > DeviceState *dev; > MemoryRegion *ecam_alias, *ecam_reg; > @@ -1113,6 +1114,7 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion > *sys_mem, > gpex_set_irq_num(GPEX_HOST(dev), i, PCIE_IRQ + i); > } > > +s->bus = PCI_HOST_BRIDGE(dev)->bus; > return dev; > } > > @@ -1502,7 +1504,7 @@ static void virt_machine_init(MachineState *machine) > virt_high_pcie_memmap.base, > virt_high_pcie_memmap.size, > memmap[VIRT_PCIE_PIO].base, > - pcie_irqchip); > + pcie_irqchip, s); > > create_platform_bus(s, mmio_irqchip); > > diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h > index e5c474b26e..4ef1f660ab 100644 > --- a/include/hw/riscv/virt.h > +++ b/include/hw/riscv/virt.h > @@ -60,6 +60,7 @@ struct RISCVVirtState { > char *oem_table_id; > OnOffAuto acpi; > const MemMapEntry *memmap; > +PCIBus *bus; > }; > > enum { > -- > 2.39.2 > >
Re: [PATCH 01/10] hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location
On Thu, Jul 13, 2023 at 2:41 AM Sunil V L wrote: > > The functions which add fw_cfg and virtio to DSDT are same for ARM > and RISC-V. So, instead of duplicating in RISC-V, move them from > hw/arm/virt-acpi-build.c to common aml-build.c. > > Signed-off-by: Sunil V L Reviewed-by: Alistair Francis Alistair > --- > hw/acpi/aml-build.c | 41 > hw/arm/virt-acpi-build.c| 42 - > hw/riscv/virt-acpi-build.c | 16 -- > include/hw/acpi/aml-build.h | 6 ++ > 4 files changed, 47 insertions(+), 58 deletions(-) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index ea331a20d1..eeb1263c8c 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -2467,3 +2467,44 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const > char *resource_source) > > return var; > } > + > +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > +{ > +Aml *dev = aml_device("FWCF"); > +aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > +/* device present, functioning, decoding, not shown in UI */ > +aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > +aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > + > +Aml *crs = aml_resource_template(); > +aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > + fw_cfg_memmap->size, AML_READ_WRITE)); > +aml_append(dev, aml_name_decl("_CRS", crs)); > +aml_append(scope, dev); > +} > + > +void acpi_dsdt_add_virtio(Aml *scope, > + const MemMapEntry *virtio_mmio_memmap, > + uint32_t mmio_irq, int num) > +{ > +hwaddr base = virtio_mmio_memmap->base; > +hwaddr size = virtio_mmio_memmap->size; > +int i; > + > +for (i = 0; i < num; i++) { > +uint32_t irq = mmio_irq + i; > +Aml *dev = aml_device("VR%02u", i); > +aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); > +aml_append(dev, aml_name_decl("_UID", aml_int(i))); > +aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > + > +Aml *crs = aml_resource_template(); > +aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); > +aml_append(crs, > + aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, > + AML_EXCLUSIVE, , 1)); > +aml_append(dev, aml_name_decl("_CRS", crs)); > +aml_append(scope, dev); > +base += size; > +} > +} > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 6b674231c2..fdedb68e2b 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -35,7 +35,6 @@ > #include "target/arm/cpu.h" > #include "hw/acpi/acpi-defs.h" > #include "hw/acpi/acpi.h" > -#include "hw/nvram/fw_cfg.h" > #include "hw/acpi/bios-linker-loader.h" > #include "hw/acpi/aml-build.h" > #include "hw/acpi/utils.h" > @@ -94,21 +93,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const > MemMapEntry *uart_memmap, > aml_append(scope, dev); > } > > -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry > *fw_cfg_memmap) > -{ > -Aml *dev = aml_device("FWCF"); > -aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > -/* device present, functioning, decoding, not shown in UI */ > -aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > -aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > -Aml *crs = aml_resource_template(); > -aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > - fw_cfg_memmap->size, AML_READ_WRITE)); > -aml_append(dev, aml_name_decl("_CRS", crs)); > -aml_append(scope, dev); > -} > - > static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap) > { > Aml *dev, *crs; > @@ -133,32 +117,6 @@ static void acpi_dsdt_add_flash(Aml *scope, const > MemMapEntry *flash_memmap) > aml_append(scope, dev); > } > > -static void acpi_dsdt_add_virtio(Aml *scope, > - const MemMapEntry *virtio_mmio_memmap, > - uint32_t mmio_irq, int num) > -{ > -hwaddr base = virtio_mmio_memmap->base; > -hwaddr size = virtio_mmio_memmap->size; > -int i; > - > -for (i = 0; i < num; i++) { > -uint32_t irq = mmio_irq + i; > -Aml *dev = aml_device("VR%02u", i); > -aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); > -aml_append(dev, aml_name_decl("_UID", aml_int(i))); > -aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > -Aml *crs = aml_resource_template(); > -aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); > -aml_append(crs, > - aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, > - AML_EXCLUSIVE, , 1)); > -
Re: [PATCH 0/2] riscv: Fix the console of the Spike machine on big endian hosts
On Fri, Jul 21, 2023 at 7:48 PM Thomas Huth wrote: > > The tests/avocado/riscv_opensbi.py avocado test is currently failing > on big endian hosts since the console of the Spike machine is not > working there. With two small patches, this can be fixed: First patch > fixes riscv64, and the second one fixes riscv32. > > Thomas Huth (2): > hw/char/riscv_htif: Fix printing of console characters on big endian > hosts > hw/char/riscv_htif: Fix the console syscall on big endian hosts Thanks! Applied to riscv-to-apply.next Alistair > > hw/char/riscv_htif.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > -- > 2.39.3 > >
Re: [PATCH] target/riscv/cpu.c: do not run 'host' CPU with TCG
On Fri, Jul 21, 2023 at 11:35 PM Daniel Henrique Barboza wrote: > > The 'host' CPU is available in a CONFIG_KVM build and it's currently > available for all accels, but is a KVM only CPU. This means that in a > RISC-V KVM capable host we can do things like this: > > $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic > qemu-system-riscv64: H extension requires priv spec 1.12.0 > > This CPU does not have a priv spec because we don't filter its extensions > via priv spec. We shouldn't be reaching riscv_cpu_realize_tcg() at all > with the 'host' CPU. > > We don't have a way to filter the 'host' CPU out of the available CPU > options (-cpu help) if the build includes both KVM and TCG. What we can > do is to error out during riscv_cpu_realize_tcg() if the user chooses > the 'host' CPU with accel=tcg: > > $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic > qemu-system-riscv64: 'host' CPU is not compatible with TCG acceleration > > Signed-off-by: Daniel Henrique Barboza Thanks! Applied to riscv-to-apply.next Alistair > --- > target/riscv/cpu.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 6b93b04453..08db3d613f 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1395,6 +1395,11 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, > Error **errp) > CPURISCVState *env = >env; > Error *local_err = NULL; > > +if (object_dynamic_cast(OBJECT(dev), TYPE_RISCV_CPU_HOST)) { > +error_setg(errp, "'host' CPU is not compatible with TCG > acceleration"); > +return; > +} > + > riscv_cpu_validate_misa_mxl(cpu, _err); > if (local_err != NULL) { > error_propagate(errp, local_err); > -- > 2.41.0 > >
Re: [PATCH 2/2] target/riscv/cpu.c: add smepmp isa string
On Thu, Jul 20, 2023 at 11:26 PM Daniel Henrique Barboza wrote: > > The cpu->cfg.epmp extension is still experimental, but it already has a > 'smepmp' riscv,isa string. Add it. > > Signed-off-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Alistair > --- > target/riscv/cpu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index d64ac07558..8c9acadd3b 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -130,6 +130,7 @@ static const struct isa_ext_data isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx), > ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), > ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia), > +ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, epmp), > ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen), > ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf), > -- > 2.41.0 > >
Re: [PATCH 1/2] target/riscv/cpu.c: add zmmul isa string
On Thu, Jul 20, 2023 at 11:25 PM Daniel Henrique Barboza wrote: > > zmmul was promoted from experimental to ratified in commit 6d00ffad4e95. > Add a riscv,isa string for it. > > Fixes: 6d00ffad4e95 ("target/riscv: move zmmul out of the experimental > properties") > Signed-off-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Alistair > --- > target/riscv/cpu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 9339c0241d..d64ac07558 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -88,6 +88,7 @@ static const struct isa_ext_data isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), > ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), > ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > +ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul), > ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs), > ISA_EXT_DATA_ENTRY(zfa, PRIV_VERSION_1_12_0, ext_zfa), > ISA_EXT_DATA_ENTRY(zfbfmin, PRIV_VERSION_1_12_0, ext_zfbfmin), > -- > 2.41.0 > >
Re: [PATCH] target/riscv/cpu.c: do not run 'host' CPU with TCG
On Fri, Jul 21, 2023 at 11:35 PM Daniel Henrique Barboza wrote: > > The 'host' CPU is available in a CONFIG_KVM build and it's currently > available for all accels, but is a KVM only CPU. This means that in a > RISC-V KVM capable host we can do things like this: > > $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic > qemu-system-riscv64: H extension requires priv spec 1.12.0 > > This CPU does not have a priv spec because we don't filter its extensions > via priv spec. We shouldn't be reaching riscv_cpu_realize_tcg() at all > with the 'host' CPU. > > We don't have a way to filter the 'host' CPU out of the available CPU > options (-cpu help) if the build includes both KVM and TCG. What we can > do is to error out during riscv_cpu_realize_tcg() if the user chooses > the 'host' CPU with accel=tcg: > > $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic > qemu-system-riscv64: 'host' CPU is not compatible with TCG acceleration > > Signed-off-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Alistair > --- > target/riscv/cpu.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 6b93b04453..08db3d613f 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1395,6 +1395,11 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, > Error **errp) > CPURISCVState *env = >env; > Error *local_err = NULL; > > +if (object_dynamic_cast(OBJECT(dev), TYPE_RISCV_CPU_HOST)) { > +error_setg(errp, "'host' CPU is not compatible with TCG > acceleration"); > +return; > +} > + > riscv_cpu_validate_misa_mxl(cpu, _err); > if (local_err != NULL) { > error_propagate(errp, local_err); > -- > 2.41.0 > >
Re: [PATCH 1/2] hw/char/riscv_htif: Fix printing of console characters on big endian hosts
On Fri, Jul 21, 2023 at 7:48 PM Thomas Huth wrote: > > The character that should be printed is stored in the 64 bit "payload" > variable. The code currently tries to print it by taking the address > of the variable and passing this pointer to qemu_chr_fe_write(). However, > this only works on little endian hosts where the least significant bits > are stored on the lowest address. To do this in a portable way, we have > to store the value in an uint8_t variable instead. > > Fixes: 5033606780 ("RISC-V HTIF Console") > Signed-off-by: Thomas Huth Reviewed-by: Alistair Francis Alistair > --- > hw/char/riscv_htif.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c > index 37d3ccc76b..f96df40124 100644 > --- a/hw/char/riscv_htif.c > +++ b/hw/char/riscv_htif.c > @@ -232,7 +232,8 @@ static void htif_handle_tohost_write(HTIFState *s, > uint64_t val_written) > s->tohost = 0; /* clear to indicate we read */ > return; > } else if (cmd == HTIF_CONSOLE_CMD_PUTC) { > -qemu_chr_fe_write(>chr, (uint8_t *), 1); > +uint8_t ch = (uint8_t)payload; > +qemu_chr_fe_write(>chr, , 1); > resp = 0x100 | (uint8_t)payload; > } else { > qemu_log("HTIF device %d: unknown command\n", device); > -- > 2.39.3 > >
Re: [PATCH 2/2] hw/char/riscv_htif: Fix the console syscall on big endian hosts
On Fri, Jul 21, 2023 at 7:48 PM Thomas Huth wrote: > > Values that have been read via cpu_physical_memory_read() from the > guest's memory have to be swapped in case the host endianess differs > from the guest. > > Fixes: a6e13e31d5 ("riscv_htif: Support console output via proxy syscall") > Signed-off-by: Thomas Huth Reviewed-by: Alistair Francis Alistair > --- > hw/char/riscv_htif.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c > index f96df40124..40de6b8b77 100644 > --- a/hw/char/riscv_htif.c > +++ b/hw/char/riscv_htif.c > @@ -30,6 +30,7 @@ > #include "qemu/timer.h" > #include "qemu/error-report.h" > #include "exec/address-spaces.h" > +#include "exec/tswap.h" > #include "sysemu/dma.h" > > #define RISCV_DEBUG_HTIF 0 > @@ -209,11 +210,11 @@ static void htif_handle_tohost_write(HTIFState *s, > uint64_t val_written) > } else { > uint64_t syscall[8]; > cpu_physical_memory_read(payload, syscall, sizeof(syscall)); > -if (syscall[0] == PK_SYS_WRITE && > -syscall[1] == HTIF_DEV_CONSOLE && > -syscall[3] == HTIF_CONSOLE_CMD_PUTC) { > +if (tswap64(syscall[0]) == PK_SYS_WRITE && > +tswap64(syscall[1]) == HTIF_DEV_CONSOLE && > +tswap64(syscall[3]) == HTIF_CONSOLE_CMD_PUTC) { > uint8_t ch; > -cpu_physical_memory_read(syscall[2], , 1); > +cpu_physical_memory_read(tswap64(syscall[2]), , 1); > qemu_chr_fe_write(>chr, , 1); > resp = 0x100 | (uint8_t)payload; > } else { > -- > 2.39.3 > >
[PATCH] Allow UNIX socket for VNC websocket
Signed-off-by: Sergii Zasenko --- ui/vnc.c | 5 - 1 file changed, 5 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 92964dc..dea1414 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char *addrstr, addr->type = SOCKET_ADDRESS_TYPE_UNIX; addr->u.q_unix.path = g_strdup(addrstr + 5); -if (websocket) { -error_setg(errp, "UNIX sockets not supported with websock"); -goto cleanup; -} - if (to) { error_setg(errp, "Port range not support with UNIX socket"); goto cleanup; -- 2.39.2
[PATCH] target/i386: Fix reporting of CPU dies when nr_cores=nr_threads=1
When QEMU is started with `-smp D,sockets=1,dies=D,cores=1,threads=1` (that is, 1 socket with D dies but each die contains just a single thread), both Linux and Windows guests incorrectly interprets the system as having D sockets with 1 die each Ultimately this is caused by various CPUID leaves not being die-aware in their "threads per socket" calculations, so this patch fixes that These changes are referenced to the AMD PPR for Family 19h Model 01h (Milan) and Family 17h Model 01h (Naples) manuals: - CPUID_Fn0001_EBX[23:16]: Number of threads in the processor (Core::X86::Cpuid::SizeId[NC] + 1) - CPUID_Fn000B_EBX_x01[15:0]: Number of logical cores in processor socket (not present until Rome) - CPUID_Fn8001_ECX[1]: Multi core product (Core::X86::Cpuid::SizeId[NC] != 0) - CPUID_Fn8008_ECX[7:0]: The number of threads in the package - 1 (Core::X86::Cpuid::SizeId[NC]) Note there are two remaining occurences that I didn't touch: - CPUID_Fn801E_ECX[10:8]: Always 0 (1 node per processor) for Milan. But for Naples, it can also be 2 or 4 nodes where each node is defined as one or two CCXes (CCD?). But Milan also has multiple CCXes, so clearly the definition of a node is different from model to model, so I've left it untouched. (QEMU seems to use the Naples definition) - MSR_CORE_THREAD_COUNT: This MSR doesn't exist on Milan or Naples Signed-off-by: 小太 --- target/i386/cpu.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 97ad229d8b..6ff23fa590 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6049,8 +6049,8 @@ 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; +if (env->nr_dies * cs->nr_cores * cs->nr_threads > 1) { +*ebx |= (env->nr_dies * cs->nr_cores * cs->nr_threads) << 16; *edx |= CPUID_HT; } if (!cpu->enable_pmu) { @@ -6230,7 +6230,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 1: *eax = apicid_pkg_offset(_info); -*ebx = cs->nr_cores * cs->nr_threads; +*ebx = env->nr_dies * cs->nr_cores * cs->nr_threads; *ecx |= CPUID_TOPOLOGY_LEVEL_CORE; break; default: @@ -6496,7 +6496,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, * discards multiple thread information if it is set. * So don't set it here for Intel to make Linux guests happy. */ -if (cs->nr_cores * cs->nr_threads > 1) { +if (env->nr_dies * cs->nr_cores * cs->nr_threads > 1) { if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 || env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 || env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) { @@ -6562,7 +6562,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *eax |= (cpu_x86_virtual_addr_width(env) << 8); } *ebx = env->features[FEAT_8000_0008_EBX]; -if (cs->nr_cores * cs->nr_threads > 1) { +if (env->nr_dies * cs->nr_cores * cs->nr_threads > 1) { /* * Bits 15:12 is "The number of bits in the initial * Core::X86::Apic::ApicId[ApicId] value that indicate @@ -6570,7 +6570,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, * Bits 7:0 is "The number of threads in the package is NC+1" */ *ecx = (apicid_pkg_offset(_info) << 12) | - ((cs->nr_cores * cs->nr_threads) - 1); + ((env->nr_dies * cs->nr_cores * cs->nr_threads) - 1); } else { *ecx = 0; } -- 2.39.2
Re: [PATCH] Wrong signed data type on pageflags_* functions - limit to 2GB memory allocation
On 7/18/23 15:50, Luca Bonissi wrote: On 32bit qemu-user targets, memory allocation failed after about 2GB due to incorrect signed (instead of the correct unsigned) "last" parameter in pageflags_find and pageflags_next functions (file accel/tcg/user-exec.c). The parameter, on 32bit targets, will be signed-extent to the 64bit final uint64_t parameters, leading to incorrect comparison on the RBTree (only the first call to mmap on the upper 2GB memory will be successful). Following the patch to fix the bug (re-submit to add "signed-off-by"): Signed-off-by: Luca Bonissi Reviewed-by: Richard Henderson Don't reply to previous patches with a new patch -- tooling doesn't handle it. I've applied this by hand. r~
Re: [PATCH] accel/tcg: Zero-pad vaddr in tlb_debug output
On 7/13/23 13:07, Anton Johansson wrote: In replacing target_ulong with vaddr and TARGET_FMT_lx with VADDR_PRIx, the zero-padding of TARGET_FMT_lx got lost. Readd 16-wide zero-padding for logging consistency. Suggested-by: Peter Maydell Signed-off-by: Anton Johansson --- accel/tcg/cputlb.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) Reviewed-by: Richard Henderson And queued, thanks. r~
Re: [PATCH for-8.0] tcg/sparc64: Disable direct linking for goto_tb
On 7/19/23 02:03, Jordan Niethe wrote: On 5/4/23 1:04 am, Richard Henderson wrote: Something is wrong with this code, and also wrong with gdb on the sparc systems to which I have access, so I cannot debug it either. Disable for now, so the release is not broken. I'm not sure if it is the entire problem but it looks like the broken code had the same race as on ppc [1] between loading TCG_REG_TB and patching and executing the direct branch. [1] https://lore.kernel.org/qemu-devel/20230717093001.13167-1-jniet...@gmail.com/#t Probably, yes. Thanks for the reminder. r~
Re: [PATCH v3 07/14] target/s390x: Fix assertion failure in VFMIN/VFMAX with type 13
On 7/19/23 23:11, Ilya Leoshkevich wrote: Type 13 is reserved, so using it should result in specification exception. Due to an off-by-1 error the code triggers an assertion at a later point in time instead. Cc:qemu-sta...@nongnu.org Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)") Reviewed-by: David Hildenbrand Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/translate_vx.c.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v3 06/14] tcg/{i386,s390x}: Add earlyclobber to the op_add2's first output
On 7/19/23 23:11, Ilya Leoshkevich wrote: i386 and s390x implementations of op_add2 require an earlyclobber, which is currently missing. This breaks VCKSM in s390x guests. E.g., on x86_64 the following op: add2_i32 tmp2,tmp3,tmp2,tmp3,tmp3,tmp2 dead: 0 2 3 4 5 pref=none,0x is translated to: addl %ebx, %r12d adcl %r12d, %ebx Introduce a new C_N1_O1_I4 constraint, and make sure that earlyclobber of aliased outputs is honored. Cc:qemu-sta...@nongnu.org Fixes: 82790a870992 ("tcg: Add markup for output requires new register") Signed-off-by: Ilya Leoshkevich --- tcg/i386/tcg-target-con-set.h | 5 - tcg/i386/tcg-target.c.inc | 2 +- tcg/s390x/tcg-target-con-set.h | 8 +--- tcg/s390x/tcg-target.c.inc | 4 ++-- tcg/tcg.c | 8 +++- 5 files changed, 19 insertions(+), 8 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v3 05/14] target/s390x: Make MC raise specification exception when class >= 16
On 7/19/23 23:11, Ilya Leoshkevich wrote: MC requires bit positions 8-11 (upper 4 bits of class) to be zeros, otherwise it must raise a specification exception. Cc:qemu-sta...@nongnu.org Fixes: 20d143e2cab8 ("s390x/tcg: Implement MONITOR CALL") Reviewed-by: David Hildenbrand Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/excp_helper.c | 2 +- target/s390x/tcg/translate.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v3 04/14] target/s390x: Fix ICM with M3=0
On 7/19/23 23:11, Ilya Leoshkevich wrote: When the mask is zero, access exceptions should still be recognized for 1 byte at the second-operand address. CC should be set to 0. Cc:qemu-sta...@nongnu.org Fixes: e023e832d0ac ("s390x: translate engine for s390x CPU") Reviewed-by: David Hildenbrand Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/translate.c | 6 ++ 1 file changed, 6 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v3 03/14] target/s390x: Fix CONVERT TO LOGICAL/FIXED with out-of-range inputs
On 7/19/23 23:11, Ilya Leoshkevich wrote: CONVERT TO LOGICAL/FIXED deviate from IEEE 754 in that they raise an inexact exception on out-of-range inputs. float_flag_invalid_cvti aligns nicely with that behavior, so convert it to S390_IEEE_MASK_INEXACT. Cc:qemu-sta...@nongnu.org Fixes: defb0e3157af ("s390x: Implement opcode helpers") Reviewed-by: David Hildenbrand Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/fpu_helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v3 02/14] target/s390x: Fix CLM with M3=0
On 7/19/23 23:11, Ilya Leoshkevich wrote: When the mask is zero, access exceptions should still be recognized for 1 byte at the second-operand address. CC should be set to 0. Reviewed-by: David Hildenbrand Cc: qemu-sta...@nongnu.org Fixes: defb0e3157af ("s390x: Implement opcode helpers") Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/mem_helper.c | 5 + 1 file changed, 5 insertions(+) diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c index f417fb1183c..d6dc8b32620 100644 --- a/target/s390x/tcg/mem_helper.c +++ b/target/s390x/tcg/mem_helper.c @@ -667,6 +667,11 @@ uint32_t HELPER(clm)(CPUS390XState *env, uint32_t r1, uint32_t mask, HELPER_LOG("%s: r1 0x%x mask 0x%x addr 0x%" PRIx64 "\n", __func__, r1, mask, addr); +if (!mask) { +/* Recognize access exceptions for the first byte */ +cpu_ldub_data_ra(env, addr, ra); +} Since s390x does not implement .do_transaction_failed, I guess this is ok. But probe_read might be clearer. Anyway, Reviewed-by: Richard Henderson r~ + while (mask) { if (mask & 8) { uint8_t d = cpu_ldub_data_ra(env, addr, ra);
Re: [PATCH v3 01/14] target/s390x: Make CKSM raise an exception if R2 is odd
On 7/19/23 23:11, Ilya Leoshkevich wrote: R2 designates an even-odd register pair; the instruction should raise a specification exception when R2 is not even. Cc: qemu-sta...@nongnu.org Fixes: e023e832d0ac ("s390x: translate engine for s390x CPU") Reviewed-by: David Hildenbrand Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/translate.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index 6661b27efa4..2f61e879878 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -1991,11 +1991,18 @@ static DisasJumpType op_cxlgb(DisasContext *s, DisasOps *o) static DisasJumpType op_cksm(DisasContext *s, DisasOps *o) { int r2 = get_field(s, r2); -TCGv_i128 pair = tcg_temp_new_i128(); -TCGv_i64 len = tcg_temp_new_i64(); +TCGv_i128 pair; +TCGv_i64 len; + +if (r2 & 1) { +gen_program_exception(s, PGM_SPECIFICATION); +return DISAS_NORETURN; +} Ideally, SPEC_r2_even would be set, handling this earlier. Perhaps adding an "in2_ra2_E" might be helpful? r~
Re: [PATCH 14/14] target/arm: Adjust PAR_EL1.SH for Device and Normal-NC memory types
On 7/14/23 16:46, Peter Maydell wrote: The PAR_EL1.SH field documents that for the cases of: * Device memory * Normal memory with both Inner and Outer Non-Cacheable the field should be 0b10 rather than whatever was in the translation table descriptor field. (In the pseudocode this is handled by PAREncodeShareability().) Perform this adjustment when assembling a PAR value. Signed-off-by: Peter Maydell --- target/arm/helper.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 13/14] target/arm/ptw: Report stage 2 fault level for stage 2 faults on stage 1 ptw
On 7/14/23 16:46, Peter Maydell wrote: When we report faults due to stage 2 faults during a stage 1 page table walk, the 'level' parameter should be the level of the walk in stage 2 that faulted, not the level of the walk in stage 1. Correct the reporting of these faults. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 12/14] target/arm/ptw: Check for block descriptors at invalid levels
On 7/14/23 16:46, Peter Maydell wrote: The architecture doesn't permit block descriptors at any arbitrary level of the page table walk; it depends on the granule size which levels are permitted. We implemented only a partial version of this check which assumes that block descriptors are valid at all levels except level 3, which meant that we wouldn't deliver the Translation fault for all cases of this sort of guest page table error. Implement the logic corresponding to the pseudocode AArch64.DecodeDescriptorType() and AArch64.BlockDescSupported(). Signed-off-by: Peter Maydell --- target/arm/ptw.c | 25 +++-- 1 file changed, 23 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 11/14] target/arm/ptw: Set attributes correctly for MMU disabled data accesses
On 7/14/23 16:46, Peter Maydell wrote: When the MMU is disabled, data accesses should be Device nGnRnE, Outer Shareable, Untagged. We handle the other cases from AArch64.S1DisabledOutput() correctly but missed this one. Device nGnRnE is memattr == 0, so the only part we were missing was that shareability should be set to 2 for both insn fetches and data accesses. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 10/14] target/arm/ptw: Drop S1Translate::out_secure
On 7/14/23 16:46, Peter Maydell wrote: We only use S1Translate::out_secure in two places, where we are setting up MemTxAttrs for a page table load. We can use arm_space_is_secure(ptw->out_space) instead, which guarantees that we're setting the MemTxAttrs secure and space fields consistently, and allows us to drop the out_secure field in S1Translate entirely. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 09/14] target/arm/ptw: Remove S1Translate::in_secure
On 7/14/23 16:46, Peter Maydell wrote: We no longer look at the in_secure field of the S1Translate struct anyway, so we can remove it and all the code which sets it. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 13 - 1 file changed, 13 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 08/14] target/arm/ptw: Remove last uses of ptw->in_secure
On 7/14/23 16:46, Peter Maydell wrote: Replace the last uses of ptw->in_secure with appropriate checks on ptw->in_space. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 07/14] target/arm/ptw: Only fold in NSTable bit effects in Secure state
On 7/14/23 16:46, Peter Maydell wrote: When we do a translation in Secure state, the NSTable bits in table descriptors may downgrade us to NonSecure; we update ptw->in_secure and ptw->in_space accordingly. We guard that check correctly with a conditional that means it's only applied for Secure stage 1 translations. However, later on in get_phys_addr_lpae() we fold the effects of the NSTable bits into the final descriptor attributes bits, and there we do it unconditionally regardless of the CPU state. That means that in Realm state (where in_secure is false) we will set bit 5 in attrs, and later use it to decide to output to non-secure space. We don't in fact need to do this folding in at all any more (since commit 2f1ff4e7b9f30c): if an NSTable bit was set then we have already set ptw->in_space to ARMSS_NonSecure, and in that situation we don't look at attrs bit 5. The only thing we still need to deal with is the real NS bit in the final descriptor word, so we can just drop the code that ORed in the NSTable bit. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 06/14] target/arm/ptw: Pass an ARMSecuritySpace to arm_hcr_el2_eff_secstate()
On 7/14/23 16:46, Peter Maydell wrote: arm_hcr_el2_eff_secstate() takes a bool secure, which it uses to determine whether EL2 is enabled in the current security state. With the advent of FEAT_RME this is no longer sufficient, because EL2 can be enabled for Secure state but not for Root, and both of those will pass 'secure == true' in the callsites in ptw.c. As it happens in all of our callsites in ptw.c we either avoid making the call or else avoid using the returned value if we're doing a translation for Root, so this is not a behaviour change even if the experimental FEAT_RME is enabled. But it is less confusing in the ptw.c code if we avoid the use of a bool secure that duplicates some of the information in the ArmSecuritySpace argument. Make arm_hcr_el2_eff_secstate() take an ARMSecuritySpace argument instead. Note that since arm_hcr_el2_eff() uses the return value from arm_security_space_below_el3() for the 'space' argument, its behaviour does not change even when at EL3 (Root security state) and it continues to tell you what EL2 would be if you were in it. Signed-off-by: Peter Maydell --- target/arm/cpu.h| 2 +- target/arm/helper.c | 7 --- target/arm/ptw.c| 13 + 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 4d6c0f95d59..3743a9e2f8a 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -2555,7 +2555,7 @@ static inline bool arm_is_el2_enabled(CPUARMState *env) * "for all purposes other than a direct read or write access of HCR_EL2." * Not included here is HCR_RW. */ -uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, bool secure); +uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, ARMSecuritySpace space); uint64_t arm_hcr_el2_eff(CPUARMState *env); uint64_t arm_hcrx_el2_eff(CPUARMState *env); diff --git a/target/arm/helper.c b/target/arm/helper.c index d08c058e424..1e45fdb47c9 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -5731,11 +5731,12 @@ static void hcr_writelow(CPUARMState *env, const ARMCPRegInfo *ri, * Bits that are not included here: * RW (read from SCR_EL3.RW as needed) */ -uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, bool secure) +uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, ARMSecuritySpace space) { uint64_t ret = env->cp15.hcr_el2; -if (!arm_is_el2_enabled_secstate(env, secure)) { +if (space == ARMSS_Root || +!arm_is_el2_enabled_secstate(env, arm_space_is_secure(space))) { /* This is confusing, as without any larger context it certainly looks wrong. @@ -230,7 +230,7 @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx, } } -hcr_el2 = arm_hcr_el2_eff_secstate(env, is_secure); +hcr_el2 = arm_hcr_el2_eff_secstate(env, space); Here, it's not clear, and could produce an "incorrect" result, though of course the value is not actually used unless mmu_idx requires it. It might be better to sink the computation down into the two cases that require it. With that, a local definition like static inline uint64_t arm_hcr_el2_ptwspace(...) { assert(space != ARMSS_Root); return arm_hcr_el2_eff_secstate(env, arm_space_is_secure(space)); } could be the thing. r~
Re: [PATCH v2 1/3] include/exec: Add WITH_MMAP_LOCK_GUARD
On 7/23/23 15:18, Peter Maydell wrote: On Sat, 22 Jul 2023 at 12:35, Richard Henderson wrote: Signed-off-by: Richard Henderson --- include/exec/exec-all.h | 10 ++ bsd-user/mmap.c | 1 + linux-user/mmap.c | 1 + 3 files changed, 12 insertions(+) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 5fa0687cd2..d02517e95f 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -629,6 +629,15 @@ void TSA_NO_TSA mmap_lock(void); void TSA_NO_TSA mmap_unlock(void); bool have_mmap_lock(void); +static inline void mmap_unlock_guard(void *unused) +{ +mmap_unlock(); +} + +#define WITH_MMAP_LOCK_GUARD()\ +for (int _mmap_lock_iter __attribute__((cleanup(mmap_unlock_guard))) \ + = (mmap_lock(), 0); _mmap_lock_iter == 0; _mmap_lock_iter = 1) All our other WITH_FOO macros seem to use g_autoptr rather than a raw attribute((cleanup)); is it worth being consistent? I didn't think it worthwhile, no, since that requires even more boilerplate. (This one also doesn't allow nested uses, I think.) It does, since each variable will shadow the next within each context. r~
Re: [PATCH v2 3/3] accel/tcg: Take mmap_lock in load_atomic*_or_exit
On Sat, 22 Jul 2023 at 12:35, Richard Henderson wrote: > > For user-only, the probe for page writability may race with another > thread's mprotect. Take the mmap_lock around the operation. This > is still faster than the start/end_exclusive fallback. > > Signed-off-by: Richard Henderson > --- > accel/tcg/ldst_atomicity.c.inc | 32 ++-- > 1 file changed, 18 insertions(+), 14 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 2/3] accel/tcg: Fix sense of read-only probes in ldst_atomicity
On Sat, 22 Jul 2023 at 12:35, Richard Henderson wrote: > > In the initial commit, cdfac37be0d, the sense of the test is incorrect, > as the -1/0 return was confusing. In bef6f008b981, we mechanically > invert all callers while changing to false/true return, preserving the > incorrectness of the test. > > Now that the return sense is sane, it's easy to see that if !write, > then the page is not modifiable (i.e. most likely read-only, with > PROT_NONE handled via SIGSEGV). > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 1/3] include/exec: Add WITH_MMAP_LOCK_GUARD
On Sat, 22 Jul 2023 at 12:35, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > include/exec/exec-all.h | 10 ++ > bsd-user/mmap.c | 1 + > linux-user/mmap.c | 1 + > 3 files changed, 12 insertions(+) > > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 5fa0687cd2..d02517e95f 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -629,6 +629,15 @@ void TSA_NO_TSA mmap_lock(void); > void TSA_NO_TSA mmap_unlock(void); > bool have_mmap_lock(void); > > +static inline void mmap_unlock_guard(void *unused) > +{ > +mmap_unlock(); > +} > + > +#define WITH_MMAP_LOCK_GUARD()\ > +for (int _mmap_lock_iter __attribute__((cleanup(mmap_unlock_guard))) \ > + = (mmap_lock(), 0); _mmap_lock_iter == 0; _mmap_lock_iter = 1) All our other WITH_FOO macros seem to use g_autoptr rather than a raw attribute((cleanup)); is it worth being consistent? (This one also doesn't allow nested uses, I think.) Either way Reviewed-by: Peter Maydell since it would be nice to fix this for the next rc. thanks -- PMM
Re: [PATCH] ppc/pegasos2: Fix reg property of 64 bit BARs in device tree
On 7/21/23 19:13, BALATON Zoltan wrote: The board firmware handles this correctly following the Open Firmware standard which we missed. This fixes 64 bit BARs when using VOF. Signed-off-by: BALATON Zoltan --- Reviewed-by: Daniel Henrique Barboza And queued. Thanks, Daniel hw/ppc/pegasos2.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c index 6475acfbed..075367d94d 100644 --- a/hw/ppc/pegasos2.c +++ b/hw/ppc/pegasos2.c @@ -781,7 +781,11 @@ static void add_pci_device(PCIBus *bus, PCIDevice *d, void *opaque) if (d->io_regions[i].type & PCI_BASE_ADDRESS_SPACE_IO) { cells[j] |= cpu_to_be32(1 << 24); } else { -cells[j] |= cpu_to_be32(2 << 24); +if (d->io_regions[i].type & PCI_BASE_ADDRESS_MEM_TYPE_64) { +cells[j] |= cpu_to_be32(3 << 24); +} else { +cells[j] |= cpu_to_be32(2 << 24); +} if (d->io_regions[i].type & PCI_BASE_ADDRESS_MEM_PREFETCH) { cells[j] |= cpu_to_be32(4 << 28); }
[PATCH v3 2/4] virtio-net: Expose MAX_VLAN
vhost-vdpa shadowed CVQ needs to know the maximum number of vlans supported by the virtio-net device, so QEMU can restore the VLAN state in a migration. Co-developed-by: Eugenio Pérez Signed-off-by: Eugenio Pérez Signed-off-by: Hawkins Jiawei --- hw/net/virtio-net.c| 2 -- include/hw/virtio/virtio-net.h | 6 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index d20d5a63cd..a32672039d 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -49,8 +49,6 @@ #define VIRTIO_NET_VM_VERSION11 -#define MAX_VLAN(1 << 12) /* Per 802.1Q definition */ - /* previously fixed value */ #define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256 #define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256 diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 5f5dcb4572..93f3bb5d97 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -38,6 +38,12 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIONet, VIRTIO_NET) /* Maximum VIRTIO_NET_CTRL_MAC_TABLE_SET unicast + multicast entries. */ #define MAC_TABLE_ENTRIES64 +/* + * The maximum number of VLANs in the VLAN filter table + * added by VIRTIO_NET_CTRL_VLAN_ADD + */ +#define MAX_VLAN(1 << 12) /* Per 802.1Q definition */ + typedef struct virtio_net_conf { uint32_t txtimer; -- 2.25.1
[PATCH v3 3/4] vdpa: Restore vlan filtering state
This patch introduces vhost_vdpa_net_load_single_vlan() and vhost_vdpa_net_load_vlan() to restore the vlan filtering state at device's startup. Co-developed-by: Eugenio Pérez Signed-off-by: Eugenio Pérez Signed-off-by: Hawkins Jiawei --- v2: - remove the extra line pointed out by Eugenio v1: https://lore.kernel.org/all/0a568cc8a8d2b750c2e09b2237e9f05cece07c3f.1689690854.git.yin31...@gmail.com/ net/vhost-vdpa.c | 48 1 file changed, 48 insertions(+) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 9795306742..347241796d 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -965,6 +965,50 @@ static int vhost_vdpa_net_load_rx(VhostVDPAState *s, return 0; } +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s, + const VirtIONet *n, + uint16_t vid) +{ +const struct iovec data = { +.iov_base = , +.iov_len = sizeof(vid), +}; +ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN, + VIRTIO_NET_CTRL_VLAN_ADD, + , 1); +if (unlikely(dev_written < 0)) { +return dev_written; +} +if (unlikely(*s->status != VIRTIO_NET_OK)) { +return -EIO; +} + +return 0; +} + +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s, +const VirtIONet *n) +{ +int r; + +if (!virtio_vdev_has_feature(>parent_obj, VIRTIO_NET_F_CTRL_VLAN)) { +return 0; +} + +for (int i = 0; i < MAX_VLAN >> 5; i++) { +for (int j = 0; n->vlans[i] && j <= 0x1f; j++) { +if (n->vlans[i] & (1U << j)) { +r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + j); +if (unlikely(r != 0)) { +return r; +} +} +} +} + +return 0; +} + static int vhost_vdpa_net_load(NetClientState *nc) { VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc); @@ -995,6 +1039,10 @@ static int vhost_vdpa_net_load(NetClientState *nc) if (unlikely(r)) { return r; } +r = vhost_vdpa_net_load_vlan(s, n); +if (unlikely(r)) { +return r; +} return 0; } -- 2.25.1
[PATCH v3 4/4] vdpa: Allow VIRTIO_NET_F_CTRL_VLAN in SVQ
Enable SVQ with VIRTIO_NET_F_CTRL_VLAN feature. Co-developed-by: Eugenio Pérez Signed-off-by: Eugenio Pérez Signed-off-by: Hawkins Jiawei --- net/vhost-vdpa.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 347241796d..73e9063fa0 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -111,6 +111,7 @@ static const uint64_t vdpa_svq_device_features = BIT_ULL(VIRTIO_NET_F_STATUS) | BIT_ULL(VIRTIO_NET_F_CTRL_VQ) | BIT_ULL(VIRTIO_NET_F_CTRL_RX) | +BIT_ULL(VIRTIO_NET_F_CTRL_VLAN) | BIT_ULL(VIRTIO_NET_F_CTRL_RX_EXTRA) | BIT_ULL(VIRTIO_NET_F_MQ) | BIT_ULL(VIRTIO_F_ANY_LAYOUT) | -- 2.25.1
[PATCH v3 1/4] virtio-net: do not reset vlan filtering at set_features
This function is called after virtio_load, so all vlan configuration is lost in migration case. Just allow all the vlan-tagged packets if vlan is not configured, and trust device reset to clear all filtered vlans. Fixes: 0b1eaa8803 ("virtio-net: Do not filter VLANs without F_CTRL_VLAN") Signed-off-by: Eugenio Pérez Reviewed-by: Hawkins Jiawei Signed-off-by: Hawkins Jiawei --- v3: - remove the extra "From" line v2: https://lore.kernel.org/all/95af0d013281282f48ad3f47f6ad1ac4ca9e52eb.1690100802.git.yin31...@gmail.com/ hw/net/virtio-net.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 7102ec4817..d20d5a63cd 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1006,9 +1006,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) vhost_net_save_acked_features(nc->peer); } -if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { -memset(n->vlans, 0, MAX_VLAN >> 3); -} else { +if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { memset(n->vlans, 0xff, MAX_VLAN >> 3); } -- 2.25.1
[PATCH v3 0/4] Vhost-vdpa Shadow Virtqueue VLAN support
This series enables shadowed CVQ to intercept VLAN commands through shadowed CVQ, update the virtio NIC device model so qemu send it in a migration, and the restore of that VLAN state in the destination. ChangeLog = v3: - remove the extra "From" line in patch 1 "virtio-net: do not reset vlan filtering at set_features" v2: https://lore.kernel.org/all/cover.1690100802.git.yin31...@gmail.com/ - remove the extra line pointed out by Eugenio in patch 3 "vdpa: Restore vlan filtering state" v1: https://lore.kernel.org/all/cover.1689690854.git.yin31...@gmail.com/ - based on patch "[PATCH 0/3] Vhost-vdpa Shadow Virtqueue VLAN support" at https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg01016.html - move `MAX_VLAN` macro to include/hw/virtio/virtio-net.h instead of net/vhost-vdpa.c - fix conflicts with the master branch TestStep 1. test the migration using vp-vdpa device - For L0 guest, boot QEMU with two virtio-net-pci net device with `ctrl_vq`, `ctrl_vlan` features on, command line like: -device virtio-net-pci,disable-legacy=on,disable-modern=off, iommu_platform=on,mq=on,ctrl_vq=on,guest_announce=off, indirect_desc=off,queue_reset=off,ctrl_vlan=on,... - For L1 guest, apply the patch series and compile the source code, start QEMU with two vdpa device with svq mode on, enable the `ctrl_vq`, `ctrl_vlan` features on, command line like: -netdev type=vhost-vdpa,x-svq=true,... -device virtio-net-pci,mq=on,guest_announce=off,ctrl_vq=on, ctrl_vlan=on,... - For L2 source guest, run the following bash command: ```bash #!/bin/sh for idx in {1..4094} do ip link add link eth0 name vlan$idx type vlan id $idx done ``` - gdb attaches the L2 dest VM and break at the vhost_vdpa_net_load_single_vlan(), and execute the following gdbscript ```gdbscript ignore 1 4094 c ``` - Execute the live migration in L2 source monitor - Result * with this series, gdb can hit the breakpoint and continue the executing without triggering any error or warning. Eugenio Pérez (1): virtio-net: do not reset vlan filtering at set_features Hawkins Jiawei (3): virtio-net: Expose MAX_VLAN vdpa: Restore vlan filtering state vdpa: Allow VIRTIO_NET_F_CTRL_VLAN in SVQ hw/net/virtio-net.c| 6 + include/hw/virtio/virtio-net.h | 6 + net/vhost-vdpa.c | 49 ++ 3 files changed, 56 insertions(+), 5 deletions(-) -- 2.25.1
Re: [PATCH for-8.2 5/6] vfio/migration: Add P2P support for VFIO migration
On 21/07/2023 14:48, Cédric Le Goater wrote: External email: Use caution opening links or attachments On 7/16/23 10:15, Avihai Horon wrote: VFIO migration uAPI defines an optional intermediate P2P quiescent state. While in the P2P quiescent state, P2P DMA transactions cannot be initiated by the device, but the device can respond to incoming ones. Additionally, all outstanding P2P transactions are guaranteed to have been completed by the time the device enters this state. The purpose of this state is to support migration of multiple devices that are doing P2P transactions between themselves. Add support for P2P migration by transitioning all the devices to the P2P quiescent state before stopping or starting the devices. Use the new VMChangeStateHandler pre_change_cb to achieve that behavior. This will allow migration of multiple VFIO devices if all of them support P2P migration. LGTM, one small comment below Signed-off-by: Avihai Horon --- docs/devel/vfio-migration.rst | 93 +-- hw/vfio/common.c | 6 ++- hw/vfio/migration.c | 58 +- hw/vfio/trace-events | 2 +- 4 files changed, 107 insertions(+), 52 deletions(-) diff --git a/docs/devel/vfio-migration.rst b/docs/devel/vfio-migration.rst index b433cb5bb2..b9c57ba651 100644 --- a/docs/devel/vfio-migration.rst +++ b/docs/devel/vfio-migration.rst @@ -23,9 +23,21 @@ and recommends that the initial bytes are sent and loaded in the destination before stopping the source VM. Enabling this migration capability will guarantee that and thus, can potentially reduce downtime even further. -Note that currently VFIO migration is supported only for a single device. This -is due to VFIO migration's lack of P2P support. However, P2P support is planned -to be added later on. +To support migration of multiple devices that are doing P2P transactions +between themselves, VFIO migration uAPI defines an intermediate P2P quiescent +state. While in the P2P quiescent state, P2P DMA transactions cannot be +initiated by the device, but the device can respond to incoming ones. +Additionally, all outstanding P2P transactions are guaranteed to have been +completed by the time the device enters this state. + +All the devices that support P2P migration are first transitioned to the P2P +quiescent state and only then are they stopped or started. This makes migration +safe P2P-wise, since starting and stopping the devices is not done atomically +for all the devices together. + +Thus, multiple VFIO devices migration is allowed only if all the devices +support P2P migration. Single VFIO device migration is allowed regardless of +P2P migration support. A detailed description of the UAPI for VFIO device migration can be found in the comment for the ``vfio_device_mig_state`` structure in the header file @@ -132,54 +144,63 @@ will be blocked. Flow of state changes during Live migration === -Below is the flow of state change during live migration. +Below is the state change flow during live migration for a VFIO device that +supports both precopy and P2P migration. The flow for devices that don't +support it is similar, except that the relevant states for precopy and P2P are +skipped. The values in the parentheses represent the VM state, the migration state, and the VFIO device state, respectively. -The text in the square brackets represents the flow if the VFIO device supports -pre-copy. Live migration save path :: - QEMU normal running state - (RUNNING, _NONE, _RUNNING) - | + QEMU normal running state + (RUNNING, _NONE, _RUNNING) + | migrate_init spawns migration_thread - Migration thread then calls each device's .save_setup() - (RUNNING, _SETUP, _RUNNING [_PRE_COPY]) - | - (RUNNING, _ACTIVE, _RUNNING [_PRE_COPY]) - If device is active, get pending_bytes by .state_pending_{estimate,exact}() - If total pending_bytes >= threshold_size, call .save_live_iterate() - [Data of VFIO device for pre-copy phase is copied] - Iterate till total pending bytes converge and are less than threshold - | - On migration completion, vCPU stops and calls .save_live_complete_precopy for - each active device. The VFIO device is then transitioned into _STOP_COPY state - (FINISH_MIGRATE, _DEVICE, _STOP_COPY) - | - For the VFIO device, iterate in .save_live_complete_precopy until - pending data is 0 - (FINISH_MIGRATE, _DEVICE, _STOP) -
Re: [PATCH for-8.2 6/6] vfio/migration: Allow migration of multiple P2P supporting devices
On 21/07/2023 15:09, Cédric Le Goater wrote: External email: Use caution opening links or attachments On 7/16/23 10:15, Avihai Horon wrote: Now that P2P support has been added to VFIO migration, allow migration of multiple devices if all of them support P2P migration. Single device migration is allowed regardless of P2P migration support. Signed-off-by: Avihai Horon Signed-off-by: Joao Martins --- hw/vfio/common.c | 26 ++ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 7c3d636025..753b320739 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -363,21 +363,31 @@ bool vfio_mig_active(void) static Error *multiple_devices_migration_blocker; -static unsigned int vfio_migratable_device_num(void) +/* + * Multiple devices migration is allowed only if all devices support P2P + * migration. Single device migration is allowed regardless of P2P migration + * support. + */ +static bool vfio_should_block_multiple_devices_migration(void) Could we revert the logic and call the routine : vfio_multiple_devices_migration_is_supported() I think it would be clearer in the callers. This is minor. Yes, of course, I will change that. Thanks! { VFIOGroup *group; VFIODevice *vbasedev; unsigned int device_num = 0; + bool all_support_p2p = true; QLIST_FOREACH(group, _group_list, next) { QLIST_FOREACH(vbasedev, >device_list, next) { if (vbasedev->migration) { device_num++; + + if (!(vbasedev->migration->mig_flags & VFIO_MIGRATION_P2P)) { + all_support_p2p = false; + } } } } - return device_num; + return !all_support_p2p && device_num > 1; } int vfio_block_multiple_devices_migration(VFIODevice *vbasedev, Error **errp) @@ -385,19 +395,19 @@ int vfio_block_multiple_devices_migration(VFIODevice *vbasedev, Error **errp) int ret; if (multiple_devices_migration_blocker || - vfio_migratable_device_num() <= 1) { + !vfio_should_block_multiple_devices_migration()) { return 0; } if (vbasedev->enable_migration == ON_OFF_AUTO_ON) { - error_setg(errp, "Migration is currently not supported with multiple " - "VFIO devices"); + error_setg(errp, "Multiple VFIO devices migration is supported only if " + "all of them support P2P migration"); return -EINVAL; } error_setg(_devices_migration_blocker, - "Migration is currently not supported with multiple " - "VFIO devices"); + "Multiple VFIO devices migration is supported only if all of " + "them support P2P migration"); ret = migrate_add_blocker(multiple_devices_migration_blocker, errp); if (ret < 0) { error_free(multiple_devices_migration_blocker); @@ -410,7 +420,7 @@ int vfio_block_multiple_devices_migration(VFIODevice *vbasedev, Error **errp) void vfio_unblock_multiple_devices_migration(void) { if (!multiple_devices_migration_blocker || - vfio_migratable_device_num() > 1) { + vfio_should_block_multiple_devices_migration()) { return; }
Re: [PATCH 04/14] target/arm/ptw: Pass ptw into get_phys_addr_pmsa*() and get_phys_addr_disabled()
On 7/14/23 16:46, Peter Maydell wrote: In commit 6d2654ffacea813916176 we created the S1Translate struct and used it to plumb through various arguments that we were previously passing one-at-a-time to get_phys_addr_v5(), get_phys_addr_v6(), and get_phys_addr_lpae(). Extend that pattern to get_phys_addr_pmsav5(), get_phys_addr_pmsav7(), get_phys_addr_pmsav8() and get_phys_addr_disabled(), so that all the get_phys_addr_* functions we call from get_phys_addr_nogpc() take the S1Translate struct rather than the mmu_idx and is_secure bool. (This refactoring is a prelude to having the called functions look at ptw->is_space rather than using an is_secure boolean.) Signed-off-by: Peter Maydell --- target/arm/ptw.c | 57 ++-- 1 file changed, 36 insertions(+), 21 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 05/14] target/arm/ptw: Pass ARMSecurityState to regime_translation_disabled()
On 7/14/23 16:46, Peter Maydell wrote: Plumb the ARMSecurityState through to regime_translation_disabled() rather than just a bool is_secure. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 03/14] target/arm/ptw: Set s1ns bit in fault info more consistently
On 7/14/23 16:46, Peter Maydell wrote: The s1ns bit in ARMMMUFaultInfo is documented as "true if we faulted on a non-secure IPA while in secure state". Both the places which look at this bit only do so after having confirmed that this is a stage 2 fault and we're dealing with Secure EL2, which leaves the ptw.c code free to set the bit to any random value in the other cases. Instead of taking advantage of that freedom, consistently make the bit be set to false for the "not a stage 2 fault for Secure EL2" cases. This removes some cases where we were using an 'is_secure' boolean and leaving the reader guessing about whether that was the right thing for Realm and Root cases. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 1/4] virtio-net: do not reset vlan filtering at set_features
On 2023/7/23 17:26, Hawkins Jiawei wrote: > From: Eugenio Pérez There was a wrong "From" line by mistake, I will send the v3 patch to fix this. Thanks! > > This function is called after virtio_load, so all vlan configuration is > lost in migration case. > > Just allow all the vlan-tagged packets if vlan is not configured, and > trust device reset to clear all filtered vlans. > > Fixes: 0b1eaa8803 ("virtio-net: Do not filter VLANs without F_CTRL_VLAN") > Signed-off-by: Eugenio Pérez > Reviewed-by: Hawkins Jiawei > Signed-off-by: Hawkins Jiawei > --- > hw/net/virtio-net.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 7102ec4817..d20d5a63cd 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -1006,9 +1006,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, > uint64_t features) > vhost_net_save_acked_features(nc->peer); > } > > -if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { > -memset(n->vlans, 0, MAX_VLAN >> 3); > -} else { > +if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { > memset(n->vlans, 0xff, MAX_VLAN >> 3); > } >
Re: [PATCH 02/14] target/arm/ptw: Don't report GPC faults on stage 1 ptw as stage2 faults
On 7/14/23 16:46, Peter Maydell wrote: In S1_ptw_translate() we set up the ARMMMUFaultInfo if the attempt to translate the page descriptor address into a physical address fails. This used to only be possible if we are doing a stage 2 ptw for that descriptor address, and so the code always sets fi->stage2 and fi->s1ptw to true. However, with FEAT_RME it is also possible for the lookup of the page descriptor address to fail because of a Granule Protection Check fault. These should not be reported as stage 2, otherwise arm_deliver_fault() will incorrectly set HPFAR_EL2. Similarly the s1ptw bit should only be set for stage 2 faults on stage 1 translation table walks, i.e. not for GPC faults. Add a comment to the the other place where we might detect a stage2-fault-on-stage-1-ptw, in arm_casq_ptw(), noting why we know in that case that it must really be a stage 2 fault and not a GPC fault. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
[PULL 0/1] riscv-to-apply queue
The following changes since commit d1181d29370a4318a9f11ea92065bea6bb159f83: Merge tag 'pull-nbd-2023-07-19' of https://repo.or.cz/qemu/ericb into staging (2023-07-20 09:54:07 +0100) are available in the Git repository at: https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20230723-3 for you to fetch changes up to dcaaf2bf9bfd2c664dbeff0069fcab3d75c924d3: roms/opensbi: Upgrade from v1.3 to v1.3.1 (2023-07-23 19:32:02 +1000) Fifth RISC-V PR for 8.1 * roms/opensbi: Upgrade from v1.3 to v1.3.1 Bin Meng (1): roms/opensbi: Upgrade from v1.3 to v1.3.1 pc-bios/opensbi-riscv32-generic-fw_dynamic.bin | Bin 135344 -> 135376 bytes pc-bios/opensbi-riscv64-generic-fw_dynamic.bin | Bin 138304 -> 138368 bytes roms/opensbi | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-)
[PULL 1/1] roms/opensbi: Upgrade from v1.3 to v1.3.1
From: Bin Meng Upgrade OpenSBI from v1.3 to v1.3.1 and the pre-built bios images which fixes the boot failure seen when using QEMU to do a direct kernel boot with Microchip Icicle Kit board machine. The v1.3.1 release includes the following commits: 0907de3 lib: sbi: fix comment indent eb736a5 lib: sbi_pmu: Avoid out of bounds access 7828eeb gpio/desginware: add Synopsys DesignWare APB GPIO support c6a3573 lib: utils: Fix sbi_hartid_to_scratch() usage in ACLINT drivers 057eb10 lib: utils/gpio: Fix RV32 compile error for designware GPIO driver Signed-off-by: Bin Meng Message-Id: <20230719165817.889465-1-bm...@tinylab.org> Reviewed-by: Alistair Francis Tested-by: Conor Dooley Signed-off-by: Alistair Francis --- .../opensbi-riscv32-generic-fw_dynamic.bin| Bin 135344 -> 135376 bytes .../opensbi-riscv64-generic-fw_dynamic.bin| Bin 138304 -> 138368 bytes roms/opensbi | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) diff --git a/pc-bios/opensbi-riscv32-generic-fw_dynamic.bin b/pc-bios/opensbi-riscv32-generic-fw_dynamic.bin index 7b6c67e0ae..9a2ba3f2a4 100644 Binary files a/pc-bios/opensbi-riscv32-generic-fw_dynamic.bin and b/pc-bios/opensbi-riscv32-generic-fw_dynamic.bin differ diff --git a/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin b/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin index 1b831b412c..5d4e812819 100644 Binary files a/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin and b/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin differ diff --git a/roms/opensbi b/roms/opensbi index 2552799a1d..057eb10b6d 16 --- a/roms/opensbi +++ b/roms/opensbi @@ -1 +1 @@ -Subproject commit 2552799a1df30a3dcd2321a8b75d61d06f5fb9fc +Subproject commit 057eb10b6d523540012e6947d5c9f63e95244e94 -- 2.40.1
[PATCH v2 3/4] vdpa: Restore vlan filtering state
This patch introduces vhost_vdpa_net_load_single_vlan() and vhost_vdpa_net_load_vlan() to restore the vlan filtering state at device's startup. Co-developed-by: Eugenio Pérez Signed-off-by: Eugenio Pérez Signed-off-by: Hawkins Jiawei --- v2: - remove the extra line pointed out by Eugenio v1: https://lore.kernel.org/all/0a568cc8a8d2b750c2e09b2237e9f05cece07c3f.1689690854.git.yin31...@gmail.com/ net/vhost-vdpa.c | 48 1 file changed, 48 insertions(+) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 9795306742..347241796d 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -965,6 +965,50 @@ static int vhost_vdpa_net_load_rx(VhostVDPAState *s, return 0; } +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s, + const VirtIONet *n, + uint16_t vid) +{ +const struct iovec data = { +.iov_base = , +.iov_len = sizeof(vid), +}; +ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN, + VIRTIO_NET_CTRL_VLAN_ADD, + , 1); +if (unlikely(dev_written < 0)) { +return dev_written; +} +if (unlikely(*s->status != VIRTIO_NET_OK)) { +return -EIO; +} + +return 0; +} + +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s, +const VirtIONet *n) +{ +int r; + +if (!virtio_vdev_has_feature(>parent_obj, VIRTIO_NET_F_CTRL_VLAN)) { +return 0; +} + +for (int i = 0; i < MAX_VLAN >> 5; i++) { +for (int j = 0; n->vlans[i] && j <= 0x1f; j++) { +if (n->vlans[i] & (1U << j)) { +r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + j); +if (unlikely(r != 0)) { +return r; +} +} +} +} + +return 0; +} + static int vhost_vdpa_net_load(NetClientState *nc) { VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc); @@ -995,6 +1039,10 @@ static int vhost_vdpa_net_load(NetClientState *nc) if (unlikely(r)) { return r; } +r = vhost_vdpa_net_load_vlan(s, n); +if (unlikely(r)) { +return r; +} return 0; } -- 2.25.1
[PATCH v2 4/4] vdpa: Allow VIRTIO_NET_F_CTRL_VLAN in SVQ
Enable SVQ with VIRTIO_NET_F_CTRL_VLAN feature. Co-developed-by: Eugenio Pérez Signed-off-by: Eugenio Pérez Signed-off-by: Hawkins Jiawei --- net/vhost-vdpa.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 347241796d..73e9063fa0 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -111,6 +111,7 @@ static const uint64_t vdpa_svq_device_features = BIT_ULL(VIRTIO_NET_F_STATUS) | BIT_ULL(VIRTIO_NET_F_CTRL_VQ) | BIT_ULL(VIRTIO_NET_F_CTRL_RX) | +BIT_ULL(VIRTIO_NET_F_CTRL_VLAN) | BIT_ULL(VIRTIO_NET_F_CTRL_RX_EXTRA) | BIT_ULL(VIRTIO_NET_F_MQ) | BIT_ULL(VIRTIO_F_ANY_LAYOUT) | -- 2.25.1
[PATCH v2 1/4] virtio-net: do not reset vlan filtering at set_features
From: Eugenio Pérez This function is called after virtio_load, so all vlan configuration is lost in migration case. Just allow all the vlan-tagged packets if vlan is not configured, and trust device reset to clear all filtered vlans. Fixes: 0b1eaa8803 ("virtio-net: Do not filter VLANs without F_CTRL_VLAN") Signed-off-by: Eugenio Pérez Reviewed-by: Hawkins Jiawei Signed-off-by: Hawkins Jiawei --- hw/net/virtio-net.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 7102ec4817..d20d5a63cd 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1006,9 +1006,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) vhost_net_save_acked_features(nc->peer); } -if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { -memset(n->vlans, 0, MAX_VLAN >> 3); -} else { +if (!virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { memset(n->vlans, 0xff, MAX_VLAN >> 3); } -- 2.25.1
[PATCH v2 2/4] virtio-net: Expose MAX_VLAN
vhost-vdpa shadowed CVQ needs to know the maximum number of vlans supported by the virtio-net device, so QEMU can restore the VLAN state in a migration. Co-developed-by: Eugenio Pérez Signed-off-by: Eugenio Pérez Signed-off-by: Hawkins Jiawei --- hw/net/virtio-net.c| 2 -- include/hw/virtio/virtio-net.h | 6 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index d20d5a63cd..a32672039d 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -49,8 +49,6 @@ #define VIRTIO_NET_VM_VERSION11 -#define MAX_VLAN(1 << 12) /* Per 802.1Q definition */ - /* previously fixed value */ #define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256 #define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256 diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 5f5dcb4572..93f3bb5d97 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -38,6 +38,12 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIONet, VIRTIO_NET) /* Maximum VIRTIO_NET_CTRL_MAC_TABLE_SET unicast + multicast entries. */ #define MAC_TABLE_ENTRIES64 +/* + * The maximum number of VLANs in the VLAN filter table + * added by VIRTIO_NET_CTRL_VLAN_ADD + */ +#define MAX_VLAN(1 << 12) /* Per 802.1Q definition */ + typedef struct virtio_net_conf { uint32_t txtimer; -- 2.25.1
[PATCH v2 0/4] Vhost-vdpa Shadow Virtqueue VLAN support
This series enables shadowed CVQ to intercept VLAN commands through shadowed CVQ, update the virtio NIC device model so qemu send it in a migration, and the restore of that VLAN state in the destination. ChangeLog = v2: - remove the extra line pointed out by Eugenio in patch 3 "vdpa: Restore vlan filtering state" v1: https://lore.kernel.org/all/cover.1689690854.git.yin31...@gmail.com/ - based on patch "[PATCH 0/3] Vhost-vdpa Shadow Virtqueue VLAN support" at https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg01016.html - move `MAX_VLAN` macro to include/hw/virtio/virtio-net.h instead of net/vhost-vdpa.c - fix conflicts with the master branch TestStep 1. test the migration using vp-vdpa device - For L0 guest, boot QEMU with two virtio-net-pci net device with `ctrl_vq`, `ctrl_vlan` features on, command line like: -device virtio-net-pci,disable-legacy=on,disable-modern=off, iommu_platform=on,mq=on,ctrl_vq=on,guest_announce=off, indirect_desc=off,queue_reset=off,ctrl_vlan=on,... - For L1 guest, apply the patch series and compile the source code, start QEMU with two vdpa device with svq mode on, enable the `ctrl_vq`, `ctrl_vlan` features on, command line like: -netdev type=vhost-vdpa,x-svq=true,... -device virtio-net-pci,mq=on,guest_announce=off,ctrl_vq=on, ctrl_vlan=on,... - For L2 source guest, run the following bash command: ```bash #!/bin/sh for idx in {1..4094} do ip link add link eth0 name vlan$idx type vlan id $idx done ``` - gdb attaches the L2 dest VM and break at the vhost_vdpa_net_load_single_vlan(), and execute the following gdbscript ```gdbscript ignore 1 4094 c ``` - Execute the live migration in L2 source monitor - Result * with this series, gdb can hit the breakpoint and continue the executing without triggering any error or warning. Eugenio Pérez (1): virtio-net: do not reset vlan filtering at set_features Hawkins Jiawei (3): virtio-net: Expose MAX_VLAN vdpa: Restore vlan filtering state vdpa: Allow VIRTIO_NET_F_CTRL_VLAN in SVQ hw/net/virtio-net.c| 6 + include/hw/virtio/virtio-net.h | 6 + net/vhost-vdpa.c | 49 ++ 3 files changed, 56 insertions(+), 5 deletions(-) -- 2.25.1
Re: [PATCH 01/14] target/arm/ptw: Don't set fi->s1ptw for UnsuppAtomicUpdate fault
On 7/14/23 16:46, Peter Maydell wrote: For an Unsupported Atomic Update fault where the stage 1 translation table descriptor update can't be done because it's to an unsupported memory type, this is a stage 1 abort (per the Arm ARM R_VSXXT). This means we should not set fi->s1ptw, because this will cause the code in the get_phys_addr_lpae() error-exit path to mark it as stage 2. Signed-off-by: Peter Maydell --- target/arm/ptw.c | 1 - 1 file changed, 1 deletion(-) Confusingly named, but correctly documented. Reviewed-by: Richard Henderson r~