Re: [PATCH for-8.2 0/4] rtc devices: Avoid putting time_t in 32-bit variables

2023-07-23 Thread Markus Armbruster
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

2023-07-23 Thread Kim, Dongwon

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

2023-07-23 Thread Alistair Francis
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[]

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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.

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Gavin Shan

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

2023-07-23 Thread Yui Washizu



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

2023-07-23 Thread Gavin Shan

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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Sergii Zasenko
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

2023-07-23 Thread 小太
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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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()

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Peter Maydell
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

2023-07-23 Thread Peter Maydell
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

2023-07-23 Thread Peter Maydell
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

2023-07-23 Thread Daniel Henrique Barboza




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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Avihai Horon



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

2023-07-23 Thread Avihai Horon



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()

2023-07-23 Thread Richard Henderson

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()

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Richard Henderson

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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Alistair Francis
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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Hawkins Jiawei
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

2023-07-23 Thread Richard Henderson

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~