[PATCH 0/2] ui/cocoa: Add cursor composition

[Akihiko Odaki]

Add accelerated cursor composition to ui/cocoa. This does not only
improve performance for display devices that exposes the capability to
the guest according to dpy_cursor_define_supported(), but fixes the
cursor display for devices that unconditionally expects the availability
of the capability (e.g., virtio-gpu).

The common pattern to implement accelerated cursor composition is to
replace the cursor and warp it so that the replaced cursor is shown at
the correct position on the guest display. Unfortunately, ui/cocoa
cannot do the same because warping the cursor position interfers with
the mouse input so it uses CALayer instead; although it is not
specialized for cursor composition, it still can compose images with
hardware acceleration.

ui/cocoa was the only non-headless graphical display lacking cursor
composition so dpy_cursor_define_supported() is no longer needed and
removed.

Signed-off-by: Akihiko Odaki 
---
Akihiko Odaki (2):
  ui/cocoa: Add cursor composition
  ui/console: Remove dpy_cursor_define_supported()

 meson.build |  3 +-
 include/ui/console.h|  1 -
 hw/display/qxl-render.c |  4 --
 hw/display/vmware_vga.c |  6 +--
 ui/console.c| 13 ---
 ui/cocoa.m  | 97 +
 6 files changed, 101 insertions(+), 23 deletions(-)
---
base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
change-id: 20240318-cursor-3491b1806582

Best regards,
-- 
Akihiko Odaki 

[PATCH 2/2] ui/console: Remove dpy_cursor_define_supported()

[Akihiko Odaki]

Remove dpy_cursor_define_supported() as it brings no benefit today and
it has a few inherent problems.

All graphical displays except egl-headless support cursor composition
without DMA-BUF, and egl-headless is meant to be used in conjunction
with another graphical display, so dpy_cursor_define_supported()
always returns true and meaningless.

Even if we add a new display without cursor composition in the future,
dpy_cursor_define_supported() will be problematic as a cursor display
fix for it because some display devices like virtio-gpu cannot tell the
lack of cursor composition capability to the guest and are unable to
utilize the value the function returns. Therefore, all non-headless
graphical displays must actually implement cursor composition for
correct cursor display.

Another problem with dpy_cursor_define_supported() is that it returns
true even if only some of the display listeners support cursor
composition, which is wrong unless all display listeners that lack
cursor composition is headless.

Signed-off-by: Akihiko Odaki 
---
 include/ui/console.h|  1 -
 hw/display/qxl-render.c |  4 
 hw/display/vmware_vga.c |  6 ++
 ui/console.c| 13 -
 4 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index a4a49ffc640c..7566282be986 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -342,7 +342,6 @@ void dpy_text_update(QemuConsole *con, int x, int y, int w, 
int h);
 void dpy_text_resize(QemuConsole *con, int w, int h);
 void dpy_mouse_set(QemuConsole *con, int x, int y, int on);
 void dpy_cursor_define(QemuConsole *con, QEMUCursor *cursor);
-bool dpy_cursor_define_supported(QemuConsole *con);
 bool dpy_gfx_check_format(QemuConsole *con,
   pixman_format_code_t format);
 
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index ec99ec887a6e..837d2446cd52 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -307,10 +307,6 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt 
*ext)
 return 1;
 }
 
-if (!dpy_cursor_define_supported(qxl->vga.con)) {
-return 0;
-}
-
 if (qxl->debug > 1 && cmd->type != QXL_CURSOR_MOVE) {
 fprintf(stderr, "%s", __func__);
 qxl_log_cmd_cursor(qxl, cmd, ext->group_id);
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 1c0f9d9a991d..fadfefc4c67c 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -904,10 +904,8 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t 
address)
 caps |= SVGA_CAP_RECT_FILL;
 #endif
 #ifdef HW_MOUSE_ACCEL
-if (dpy_cursor_define_supported(s->vga.con)) {
-caps |= SVGA_CAP_CURSOR | SVGA_CAP_CURSOR_BYPASS_2 |
-SVGA_CAP_CURSOR_BYPASS;
-}
+caps |= SVGA_CAP_CURSOR | SVGA_CAP_CURSOR_BYPASS_2 |
+SVGA_CAP_CURSOR_BYPASS;
 #endif
 ret = caps;
 break;
diff --git a/ui/console.c b/ui/console.c
index 832055675c50..6b6f57441c53 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1058,19 +1058,6 @@ void dpy_cursor_define(QemuConsole *c, QEMUCursor 
*cursor)
 }
 }
 
-bool dpy_cursor_define_supported(QemuConsole *con)
-{
-DisplayState *s = con->ds;
-DisplayChangeListener *dcl;
-
-QLIST_FOREACH(dcl, >listeners, next) {
-if (dcl->ops->dpy_cursor_define) {
-return true;
-}
-}
-return false;
-}
-
 QEMUGLContext dpy_gl_ctx_create(QemuConsole *con,
 struct QEMUGLParams *qparams)
 {

-- 
2.44.0

Re: [PATCH-for-9.1 02/21] target/mips: Declare CPU QOM types using DEFINE_TYPES() macro

[Zhao Liu]

On Fri, Mar 15, 2024 at 02:08:50PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:50 +0100
> From: Philippe Mathieu-Daudé 
> Subject: [PATCH-for-9.1 02/21] target/mips: Declare CPU QOM types using
>  DEFINE_TYPES() macro
> X-Mailer: git-send-email 2.41.0
> 
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. In
> particular because type array declared with such macro
> are easier to review.
> 
> In few commits we are going to add more types, so replace
> the type_register_static() to ease further reviews.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Richard Henderson 
> Message-Id: <20231013140116.255-15-phi...@linaro.org>
> ---
>  target/mips/cpu.c | 23 +--
>  1 file changed, 13 insertions(+), 10 deletions(-)

Reviewed-by: Zhao Liu 

Re: Regression in v7.2.10 - ui-dbus.so requires -fPIC

[Olaf Hering]

Mon, 18 Mar 2024 11:35:54 +0400 Marc-André Lureau :

> Alternatively, we could always build with pic: true (or pic:
> enable_modules), but that's not ideal either.

I'm sure that unconditionally building with -fPIC has no downsides in this 
context.


Olaf


pgpGUQPNKGONs.pgp
Description: Digitale Signatur von OpenPGP

Re: [PATCH 03/14] hw/core/machine-smp: Simplify variables' initialization in machine_parse_smp_config()

[Zhao Liu]

On Wed, Mar 13, 2024 at 04:22:30PM +0530, Prasad Pandit wrote:
> Date: Wed, 13 Mar 2024 16:22:30 +0530
> From: Prasad Pandit 
> Subject: Re: [PATCH 03/14] hw/core/machine-smp: Simplify variables'
>  initialization in machine_parse_smp_config()
> 
> Hello Zhao,
> 
> > (Communicating with you also helped me to understand the QAPI related 
> > parts.)
> 
> * I'm also visiting QAPI code parts for the first time. Thanks to you. :)
> 
> On Mon, 11 Mar 2024 at 10:36, Zhao Liu  wrote:
> > SMPConfiguration is created and set in machine_set_smp().
> > Firstly, it is created by g_autoptr(), and then it is initialized by
> > visit_type_SMPConfiguration().
> >
> > The visit_type_SMPConfiguration() is generated by
> > gen_visit_object_members() in scripts/qapi/visit.py.
> >
> > IIUC, in visit_type_SMPConfiguration() (let's have a look at
> > gen_visit_object_members()), there's no explicit initialization of
> > SMPConfiguration() (i.e., the parameter named "%(c_name)s *obj" in
> > gen_visit_object_members()). For int type, has_* means that the field is
> > set.
> >
> 
> * Thank you for the above details, appreciate it. I added few
> fprintf() calls to visit_type_SMPConfiguration() to see what user
> values it receives
> ===
> $ ./qemu-system-x86_64 -smp cores=2,maxcpus=2,cpus=1,sockets=2
> visit_type_SMPConfiguration: name: smp
> has_cpus: 1:1
>  has_drawvers: 0:0
>   has_books: 0:0
>   has_sockets: 1:2
> has_dies: 0:0
>  has_clusters: 0:0
>  has_cores: 1:2
>   has_threads: 0:0
> has_maxcpus: 1:2
> qemu-system-x86_64: Invalid CPU topology: product of the hierarchy
> must match maxcpus: sockets (2) * dies (1) * cores (2) * threads (0)
> != maxcpus (2)
> ===
> * As seen above, undefined -smp fields (both has_ and ) are
> set to zero(0).
> 
> ===
> main
>  qemu_init
>   qemu_apply_machine_options
>object_set_properties_from_keyval
> object_set_properties_from_qdict
>  object_property_set
>   machine_set_smp
>visit_type_SMPConfiguration
> visit_start_struct
> (gdb) p v->start_struct
> $4 = ... 0x570248e4 
> (gdb)
> (gdb)
>  qobject_input_start_struct
>if (obj) {
> *obj = g_malloc0(size);
> }
> ===
> * Stepping through function calls in gdb(1) revealed above call
> sequence which leads to  'SMPConfiguration **'  objects allocation by
> g_malloc0.

Thanks! I misunderstood, it turns out that the initialization is done here.

> 
> > This means if user doesn't initialize some field, the the value should
> > be considerred as unreliable. And I guess for int, the default
> > initialization value is the same as if we had declared a regular integer
> > variable. But anyway, fields that are not explicitly initialized should
> > not be accessed.
> 
> * g_malloc0() allocating 'SMPConfiguration **' object above assures us
> that undefined -smp fields shall always be zero(0).
> 
> * 'obj->has_' field is set only if the user has supplied the
> variable value, otherwise it remains set to zero(0).
>visit_type_SMPConfiguration_members
>  ->visit_optional
>->v->optional
> -> qobject_input_optional

Yes, you're right!

> 
> * Overall, I think there is scope to simplify the
> 'machine_parse_smp_config()' function by using SMPConfiguration
> field(s) ones.

Indeed, as you say, these items are initialized to 0 by default.

I think, however, that the initialization is so far away from where the
smp is currently parsed that one can't easily confirm it (thanks for
your confirmation!).

>From a code readability view, the fact that we're explicitly
initializing to 0 again here brings little overhead, but makes the code
more readable as well as easier to maintain. I think the small redundancy
here is worth it.

Also, in other use cases people always relies on fields marked by has_*,
and there is no (or less?) precedent for direct access to places where
has_* is not set. I understand that this is also a habit, i.e., fields
with a has_* of False by default are unreliable and avoid going directly
to them.

Regards,
Zhao

[PATCH 1/3] ui/cocoa: Fix aspect ratio

[Akihiko Odaki]

[NSWindow setContentAspectRatio:] does not trigger window resize itself,
so the wrong aspect ratio will persist if nothing resizes the window.
Call [NSWindow setContentSize:] in such a case.

Fixes: 91aa508d0274 ("ui/cocoa: Let the platform toggle fullscreen")
Signed-off-by: Akihiko Odaki 
---
 ui/cocoa.m | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index fa879d7dcd4b..d6a5b462f78b 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -508,6 +508,25 @@ - (void) drawRect:(NSRect) rect
 }
 }
 
+- (NSSize)fixAspectRatio:(NSSize)original
+{
+NSSize scaled;
+NSSize fixed;
+
+scaled.width = screen.width * original.height;
+scaled.height = screen.height * original.width;
+
+if (scaled.width < scaled.height) {
+fixed.width = scaled.width / screen.height;
+fixed.height = original.height;
+} else {
+fixed.width = original.width;
+fixed.height = scaled.height / screen.width;
+}
+
+return fixed;
+}
+
 - (NSSize) screenSafeAreaSize
 {
 NSSize size = [[[self window] screen] frame].size;
@@ -525,8 +544,10 @@ - (void) resizeWindow
 [[self window] setContentSize:NSMakeSize(screen.width, screen.height)];
 [[self window] center];
 } else if ([[self window] styleMask] & NSWindowStyleMaskFullScreen) {
-[[self window] setContentSize:[self screenSafeAreaSize]];
+[[self window] setContentSize:[self fixAspectRatio:[self 
screenSafeAreaSize]]];
 [[self window] center];
+} else {
+[[self window] setContentSize:[self fixAspectRatio:[self frame].size]];
 }
 }
 

-- 
2.44.0

[PATCH 2/3] ui/cocoa: Resize window after toggling zoom-to-fit

[Akihiko Odaki]

Resize the window so that the content will fit without zooming.

Fixes: 91aa508d0274 ("ui/cocoa: Let the platform toggle fullscreen")
Signed-off-by: Akihiko Odaki 
---
 ui/cocoa.m | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index d6a5b462f78b..1324be6d32fe 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1378,6 +1378,7 @@ - (void)zoomToFit:(id) sender
 
 [[cocoaView window] setStyleMask:styleMask];
 [sender setState:styleMask & NSWindowStyleMaskResizable ? 
NSControlStateValueOn : NSControlStateValueOff];
+[cocoaView resizeWindow];
 }
 
 - (void)toggleZoomInterpolation:(id) sender

-- 
2.44.0

[PATCH 3/3] ui/cocoa: Use NSTrackingInVisibleRect

[Akihiko Odaki]

I observed [NSTrackingArea rect] becomes de-synchronized with the view
frame with some unknown condition. Specify NSTrackingInVisibleRect
option to let Cocoa automatically update NSTrackingArea, which also
saves code for synchronization.

Fixes: 91aa508d0274 ("ui/cocoa: Let the platform toggle fullscreen")
Signed-off-by: Akihiko Odaki 
---
 ui/cocoa.m | 48 ++--
 1 file changed, 14 insertions(+), 34 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 1324be6d32fe..1e2b7d931905 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -306,7 +306,6 @@ static void handleAnyDeviceErrors(Error * err)
 */
 @interface QemuCocoaView : NSView
 {
-NSTrackingArea *trackingArea;
 QEMUScreen screen;
 pixman_image_t *pixman_image;
 QKbdState *kbd;
@@ -359,6 +358,19 @@ - (id)initWithFrame:(NSRect)frameRect
 self = [super initWithFrame:frameRect];
 if (self) {
 
+NSTrackingAreaOptions options = NSTrackingActiveInKeyWindow |
+NSTrackingMouseEnteredAndExited |
+NSTrackingMouseMoved |
+NSTrackingInVisibleRect;
+
+NSTrackingArea *trackingArea =
+[[NSTrackingArea alloc] initWithRect:CGRectZero
+ options:options
+   owner:self
+userInfo:nil];
+
+[self addTrackingArea:trackingArea];
+[trackingArea release];
 screen.width = frameRect.size.width;
 screen.height = frameRect.size.height;
 kbd = qkbd_state_init(dcl.con);
@@ -392,41 +404,9 @@ - (BOOL) isOpaque
 return YES;
 }
 
-- (void) removeTrackingRect
-{
-if (trackingArea) {
-[self removeTrackingArea:trackingArea];
-[trackingArea release];
-trackingArea = nil;
-}
-}
-
-- (void) frameUpdated
-{
-[self removeTrackingRect];
-
-if ([self window]) {
-NSTrackingAreaOptions options = NSTrackingActiveInKeyWindow |
-NSTrackingMouseEnteredAndExited |
-NSTrackingMouseMoved;
-trackingArea = [[NSTrackingArea alloc] initWithRect:[self frame]
-options:options
-  owner:self
-   userInfo:nil];
-[self addTrackingArea:trackingArea];
-[self updateUIInfo];
-}
-}
-
 - (void) viewDidMoveToWindow
 {
 [self resizeWindow];
-[self frameUpdated];
-}
-
-- (void) viewWillMoveToWindow:(NSWindow *)newWindow
-{
-[self removeTrackingRect];
 }
 
 - (void) hideCursor
@@ -1284,7 +1264,7 @@ - (void)windowDidExitFullScreen:(NSNotification 
*)notification
 - (void)windowDidResize:(NSNotification *)notification
 {
 [cocoaView updateBounds];
-[cocoaView frameUpdated];
+[cocoaView updateUIInfo];
 }
 
 /* Called when the user clicks on a window's close button */

-- 
2.44.0

[PATCH 0/3] Fixes for "ui/cocoa: Let the platform toggle fullscreen"

[Akihiko Odaki]

This series contains patches for regressions caused by commit 91aa508d0274
("ui/cocoa: Let the platform toggle fullscreen").

Signed-off-by: Akihiko Odaki 
---
Akihiko Odaki (3):
  ui/cocoa: Fix aspect ratio
  ui/cocoa: Resize window after toggling zoom-to-fit
  ui/cocoa: Use NSTrackingInVisibleRect

 ui/cocoa.m | 72 --
 1 file changed, 37 insertions(+), 35 deletions(-)
---
base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
change-id: 20240318-fixes-7b187ec236a0

Best regards,
-- 
Akihiko Odaki 

Re: [PATCH 2/4] ui/vnc: Do not use console_select()

[Marc-André Lureau]

Hi

On Mon, Mar 18, 2024 at 11:58 AM Akihiko Odaki  wrote:
>
> console_select() is shared by other displays and a console_select() call
> from one of them triggers console switching also in ui/curses,
> circumventing key state reinitialization that needs to be performed in
> preparation and resulting in stuck keys.
>
> Use its internal state to track the current active console to prevent
> such a surprise console switch.
>
> Signed-off-by: Akihiko Odaki 
> ---
>  include/ui/console.h   |  1 +
>  include/ui/kbd-state.h | 11 +++
>  ui/console.c   | 12 
>  ui/kbd-state.c |  6 ++
>  ui/vnc.c   | 14 +-
>  5 files changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index a4a49ffc640c..a703f7466499 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -413,6 +413,7 @@ void qemu_console_early_init(void);
>
>  void qemu_console_set_display_gl_ctx(QemuConsole *con, DisplayGLCtx *ctx);
>
> +QemuConsole *qemu_console_lookup_first_graphic_console(void);
>  QemuConsole *qemu_console_lookup_by_index(unsigned int index);
>  QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head);
>  QemuConsole *qemu_console_lookup_by_device_name(const char *device_id,
> diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
> index fb79776128cf..1f37b932eb62 100644
> --- a/include/ui/kbd-state.h
> +++ b/include/ui/kbd-state.h
> @@ -99,4 +99,15 @@ bool qkbd_state_modifier_get(QKbdState *kbd, QKbdModifier 
> mod);
>   */
>  void qkbd_state_lift_all_keys(QKbdState *kbd);
>
> +/**
> + * qkbd_state_switch_console: Switch console.
> + *
> + * This sends key up events to the previous console for all keys which are in
> + * down state to prevent keys being stuck, and remembers the new console.
> + *
> + * @kbd: state tracker state.
> + * @con: new QemuConsole for this state tracker.
> + */
> +void qkbd_state_switch_console(QKbdState *kbd, QemuConsole *con);
> +
>  #endif /* QEMU_UI_KBD_STATE_H */
> diff --git a/ui/console.c b/ui/console.c
> index 832055675c50..6bf02a23414c 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1325,6 +1325,18 @@ void graphic_console_close(QemuConsole *con)
>  dpy_gfx_replace_surface(con, surface);
>  }
>
> +QemuConsole *qemu_console_lookup_first_graphic_console(void)
> +{
> +QemuConsole *con;
> +
> +QTAILQ_FOREACH(con, , next) {
> +if (QEMU_IS_GRAPHIC_CONSOLE(con)) {
> +return con;
> +}
> +}
> +return NULL;
> +}
> +
>  QemuConsole *qemu_console_lookup_by_index(unsigned int index)
>  {
>  QemuConsole *con;
> diff --git a/ui/kbd-state.c b/ui/kbd-state.c
> index 62d42a7a22e1..52ed28b8a89b 100644
> --- a/ui/kbd-state.c
> +++ b/ui/kbd-state.c
> @@ -117,6 +117,12 @@ void qkbd_state_lift_all_keys(QKbdState *kbd)
>  }
>  }
>
> +void qkbd_state_switch_console(QKbdState *kbd, QemuConsole *con)
> +{
> +qkbd_state_lift_all_keys(kbd);
> +kbd->con = con;
> +}
> +
>  void qkbd_state_set_delay(QKbdState *kbd, int delay_ms)
>  {
>  kbd->key_delay_ms = delay_ms;
> diff --git a/ui/vnc.c b/ui/vnc.c
> index fc12b343e254..94564b196ba8 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1872,12 +1872,16 @@ static void do_key_event(VncState *vs, int down, int 
> keycode, int sym)
>  /* QEMU console switch */
>  switch (qcode) {
>  case Q_KEY_CODE_1 ... Q_KEY_CODE_9: /* '1' to '9' keys */
> -if (vs->vd->dcl.con == NULL && down &&
> +if (down &&
>  qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_CTRL) &&
>  qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_ALT)) {
> -/* Reset the modifiers sent to the current console */
> -qkbd_state_lift_all_keys(vs->vd->kbd);
> -console_select(qcode - Q_KEY_CODE_1);
> +QemuConsole *con = qemu_console_lookup_by_index(qcode - 
> Q_KEY_CODE_1);
> +if (con) {
> +unregister_displaychangelistener(>vd->dcl);
> +qkbd_state_switch_console(vs->vd->kbd, con);
> +vs->vd->dcl.con = con;
> +register_displaychangelistener(>vd->dcl);
> +}
>  return;
>  }
>  default:
> @@ -4206,7 +4210,7 @@ void vnc_display_open(const char *id, Error **errp)
>  goto fail;
>  }
>  } else {
> -con = NULL;
> +con = qemu_console_lookup_first_graphic_console();

why this change here?

otherwise, lgtm

>  }
>
>  if (con != vd->dcl.con) {
>
> --
> 2.44.0
>
>


-- 
Marc-André Lureau

Re: [PATCH 0/3] 64 Bit support for hppa gdbstub

[Sven Schnelle]

Hi Richard,

Sven Schnelle  writes:

> Hi List,
>
> this patchset allows to debug the hppa target when running in wide (64 bit)
> mode. gdb needs a small patch to switch to 64 bit mode. I pushed the
> patch to 
> https://github.com/bminor/binutils-gdb/commit/fd8662ec282d688d1f8100b4365823e57516857b
> With this patch gdb will switch to the appropriate mode when connecting
> to qemu/gdbstub.
>
> Sven Schnelle (3):
>   Revert "target/hppa: Drop attempted gdbstub support for hppa64"
>   target/hppa: add 64 bit support to gdbstub
>   target/hppa: mask CR_SAR register writes to 5/6 bit in gdbstub
>
>  target/hppa/gdbstub.c | 66 +--
>  1 file changed, 45 insertions(+), 21 deletions(-)

gentle ping - if i followed correctly only one patch was reviewed so far.

[PATCH 3/5] hw/loongarch: Refine fwcfg memory map

[Bibo Mao]

Memory map table for fwcfg is used for UEFI BIOS, UEFI BIOS uses the first
entry from fwcfg memory map as the first memory HOB, the second memory HOB
will be used if the first memory HOB is used up.

Memory map table for fwcfg does not care about numa node, however in
generic the first memory HOB is part of numa node0, so that runtime
memory of UEFI which is allocated from the first memory HOB is located
at numa node0.

Signed-off-by: Bibo Mao 
---
 hw/loongarch/virt.c | 60 ++---
 1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index ae79b49774..d7e0886c7c 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -796,6 +796,62 @@ static void fw_cfg_add_kernel_info(const struct 
loaderparams *loaderparams,
 }
 }
 
+static void fw_cfg_add_memory(MachineState *ms)
+{
+hwaddr base, size, ram_size, gap;
+int nb_numa_nodes, nodes;
+NodeInfo *numa_info;
+
+ram_size = ms->ram_size;
+base = VIRT_LOWMEM_BASE;
+gap = VIRT_LOWMEM_SIZE;
+nodes = nb_numa_nodes = ms->numa_state->num_nodes;
+numa_info = ms->numa_state->nodes;
+if (!nodes) {
+nodes = 1;
+}
+
+/* add fw_cfg memory map of node0 */
+if (nb_numa_nodes) {
+size = numa_info[0].node_mem;
+} else {
+size = ram_size;
+}
+
+if (size >= gap) {
+memmap_add_entry(base, gap, 1);
+size -= gap;
+base = VIRT_HIGHMEM_BASE;
+gap = ram_size - VIRT_LOWMEM_SIZE;
+}
+
+if (size) {
+memmap_add_entry(base, size, 1);
+base += size;
+}
+
+if (nodes < 2) {
+return;
+}
+
+/* add fw_cfg memory map of other nodes */
+size = ram_size - numa_info[0].node_mem;
+gap  = VIRT_LOWMEM_BASE + VIRT_LOWMEM_SIZE;
+if (base < gap && (base + size) > gap) {
+/*
+ * memory map for the maining nodes splited into two part
+ *   lowram:  [base, +(gap - base))
+ *   highram: [VIRT_HIGHMEM_BASE, +(size - (gap - base)))
+ */
+memmap_add_entry(base, gap - base, 1);
+size -= gap - base;
+base = VIRT_HIGHMEM_BASE;
+}
+
+if (size)
+memmap_add_entry(base, size, 1);
+}
+
 static void loongarch_firmware_boot(LoongArchMachineState *lams,
 const struct loaderparams *loaderparams)
 {
@@ -907,9 +963,9 @@ static void loongarch_init(MachineState *machine)
 fdt_add_cpu_nodes(lams);
 
 fdt_add_memory_nodes(machine);
+fw_cfg_add_memory(machine);
 
 /* Node0 memory */
-memmap_add_entry(VIRT_LOWMEM_BASE, VIRT_LOWMEM_SIZE, 1);
 memory_region_init_alias(>lowmem, NULL, "loongarch.node0.lowram",
  machine->ram, offset, VIRT_LOWMEM_SIZE);
 memory_region_add_subregion(address_space_mem, phyAddr, >lowmem);
@@ -922,7 +978,6 @@ static void loongarch_init(MachineState *machine)
 highram_size = ram_size - VIRT_LOWMEM_SIZE;
 }
 phyAddr = VIRT_HIGHMEM_BASE;
-memmap_add_entry(phyAddr, highram_size, 1);
 memory_region_init_alias(>highmem, NULL, "loongarch.node0.highram",
   machine->ram, offset, highram_size);
 memory_region_add_subregion(address_space_mem, phyAddr, >highmem);
@@ -937,7 +992,6 @@ static void loongarch_init(MachineState *machine)
 memory_region_init_alias(nodemem, NULL, ramName, machine->ram,
  offset,  numa_info[i].node_mem);
 memory_region_add_subregion(address_space_mem, phyAddr, nodemem);
-memmap_add_entry(phyAddr, numa_info[i].node_mem, 1);
 offset += numa_info[i].node_mem;
 phyAddr += numa_info[i].node_mem;
 }
-- 
2.39.3

[PATCH 0/5] hw/loongarch: Refine numa memory map

[Bibo Mao]

One LoongArch virt machine platform, there is limitation for memory
map information. The minimum memory size is 256M and minimum memory
size for numa node0 is 256M also. With qemu numa qtest, it is possible
that memory size of numa node0 is 128M.

Limitations for minimum memory size for both total memory and numa
node0 is removed here, including acpi srat table, fadt memory map table
and fw_cfg memory map table.

Also remove numa node about memory region, there is only low memory
region and how memory region.

Bibo Mao (5):
  hw/loongarch: Refine acpi srat table for numa memory
  hw/loongarch: Refine fadt memory table for numa memory
  hw/loongarch: Refine fwcfg memory map
  hw/loongarch: Refine system dram memory region
  hw/loongarch: Remove minimum and default memory size

 hw/loongarch/acpi-build.c |  58 +++--
 hw/loongarch/virt.c   | 168 ++
 2 files changed, 152 insertions(+), 74 deletions(-)


base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
-- 
2.39.3

[PATCH 4/5] hw/loongarch: Refine system dram memory region

[Bibo Mao]

For system dram memory region, it is not necessary to use numa node
information. There is only low memory region and high memory region.

Remove numa node information for ddr memory region here, it can reduce
memory region number about LoongArch virt machine.

Signed-off-by: Bibo Mao 
---
 hw/loongarch/virt.c | 56 ++---
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index d7e0886c7c..a2975c3c5a 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -917,19 +917,13 @@ static void loongarch_init(MachineState *machine)
 {
 LoongArchCPU *lacpu;
 const char *cpu_model = machine->cpu_type;
-ram_addr_t offset = 0;
-ram_addr_t ram_size = machine->ram_size;
-uint64_t highram_size = 0, phyAddr = 0;
 MemoryRegion *address_space_mem = get_system_memory();
 LoongArchMachineState *lams = LOONGARCH_MACHINE(machine);
-int nb_numa_nodes = machine->numa_state->num_nodes;
-NodeInfo *numa_info = machine->numa_state->nodes;
 int i;
-hwaddr fdt_base;
+hwaddr fdt_base, base, size, ram_size = machine->ram_size;
 const CPUArchIdList *possible_cpus;
 MachineClass *mc = MACHINE_GET_CLASS(machine);
 CPUState *cpu;
-char *ramName = NULL;
 struct loaderparams loaderparams = { };
 
 if (!cpu_model) {
@@ -965,41 +959,27 @@ static void loongarch_init(MachineState *machine)
 fdt_add_memory_nodes(machine);
 fw_cfg_add_memory(machine);
 
-/* Node0 memory */
-memory_region_init_alias(>lowmem, NULL, "loongarch.node0.lowram",
- machine->ram, offset, VIRT_LOWMEM_SIZE);
-memory_region_add_subregion(address_space_mem, phyAddr, >lowmem);
-
-offset += VIRT_LOWMEM_SIZE;
-if (nb_numa_nodes > 0) {
-assert(numa_info[0].node_mem > VIRT_LOWMEM_SIZE);
-highram_size = numa_info[0].node_mem - VIRT_LOWMEM_SIZE;
-} else {
-highram_size = ram_size - VIRT_LOWMEM_SIZE;
+size = ram_size;
+base = VIRT_LOWMEM_BASE;
+if (size > VIRT_LOWMEM_SIZE) {
+size = VIRT_LOWMEM_SIZE;
 }
-phyAddr = VIRT_HIGHMEM_BASE;
-memory_region_init_alias(>highmem, NULL, "loongarch.node0.highram",
-  machine->ram, offset, highram_size);
-memory_region_add_subregion(address_space_mem, phyAddr, >highmem);
-
-/* Node1 - Nodemax memory */
-offset += highram_size;
-phyAddr += highram_size;
-
-for (i = 1; i < nb_numa_nodes; i++) {
-MemoryRegion *nodemem = g_new(MemoryRegion, 1);
-ramName = g_strdup_printf("loongarch.node%d.ram", i);
-memory_region_init_alias(nodemem, NULL, ramName, machine->ram,
- offset,  numa_info[i].node_mem);
-memory_region_add_subregion(address_space_mem, phyAddr, nodemem);
-offset += numa_info[i].node_mem;
-phyAddr += numa_info[i].node_mem;
+
+memory_region_init_alias(>lowmem, NULL, "loongarch.lowram",
+  machine->ram, base, size);
+memory_region_add_subregion(address_space_mem, base, >lowmem);
+base += size;
+if (ram_size - size) {
+base = VIRT_HIGHMEM_BASE;
+memory_region_init_alias(>highmem, NULL, "loongarch.highram",
+machine->ram, VIRT_LOWMEM_BASE + size, ram_size - size);
+memory_region_add_subregion(address_space_mem, base, >highmem);
+base += ram_size - size;
 }
 
 /* initialize device memory address space */
 if (machine->ram_size < machine->maxram_size) {
 ram_addr_t device_mem_size = machine->maxram_size - machine->ram_size;
-hwaddr device_mem_base;
 
 if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
 error_report("unsupported amount of memory slots: %"PRIu64,
@@ -1013,9 +993,7 @@ static void loongarch_init(MachineState *machine)
  "%d bytes", TARGET_PAGE_SIZE);
 exit(EXIT_FAILURE);
 }
-/* device memory base is the top of high memory address. */
-device_mem_base = ROUND_UP(VIRT_HIGHMEM_BASE + highram_size, 1 * GiB);
-machine_memory_devices_init(machine, device_mem_base, device_mem_size);
+machine_memory_devices_init(machine, base, device_mem_size);
 }
 
 /* load the BIOS image. */
-- 
2.39.3

Re: [PATCH-for-9.1 04/21] target/sparc: Declare CPU QOM types using DEFINE_TYPES() macro

[Zhao Liu]

On Fri, Mar 15, 2024 at 02:08:52PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:52 +0100
> From: Philippe Mathieu-Daudé 
> Subject: [PATCH-for-9.1 04/21] target/sparc: Declare CPU QOM types using
>  DEFINE_TYPES() macro
> X-Mailer: git-send-email 2.41.0
> 
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. In
> particular because type array declared with such macro
> are easier to review.
> 
> In few commits we are going to add more types, so replace
> the type_register_static() to ease further reviews.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Mark Cave-Ayland 
> Message-Id: <20231013140116.255-17-phi...@linaro.org>
> ---
>  target/sparc/cpu.c | 23 +--
>  1 file changed, 13 insertions(+), 10 deletions(-)

Reviewed-by: Zhao Liu 

[PATCH 5/5] hw/loongarch: Remove minimum and default memory size

[Bibo Mao]

Some qtest test cases such as numa use default memory size of generic
machine class, which is 128M by fault.

Here generic default memory size is used, and also remove minimum memory
size which is 1G originally.

Signed-off-by: Bibo Mao 
---
 hw/loongarch/virt.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index a2975c3c5a..deb3750d81 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -930,10 +930,6 @@ static void loongarch_init(MachineState *machine)
 cpu_model = LOONGARCH_CPU_TYPE_NAME("la464");
 }
 
-if (ram_size < 1 * GiB) {
-error_report("ram_size must be greater than 1G.");
-exit(1);
-}
 create_fdt(lams);
 
 /* Create IOCSR space */
@@ -1241,7 +1237,6 @@ static void loongarch_class_init(ObjectClass *oc, void 
*data)
 
 mc->desc = "Loongson-3A5000 LS7A1000 machine";
 mc->init = loongarch_init;
-mc->default_ram_size = 1 * GiB;
 mc->default_cpu_type = LOONGARCH_CPU_TYPE_NAME("la464");
 mc->default_ram_id = "loongarch.ram";
 mc->max_cpus = LOONGARCH_MAX_CPUS;
-- 
2.39.3

[PATCH 2/5] hw/loongarch: Refine fadt memory table for numa memory

[Bibo Mao]

One LoongArch virt machine platform, there is limitation for memory
map information. The minimum memory size is 256M and minimum memory
size for numa node0 is 256M also. With qemu numa qtest, it is possible
that memory size of numa node0 is 128M.

Limitations for minimum memory size for both total memory and numa
node0 is removed for fadt numa memory table creation.

Signed-off-by: Bibo Mao 
---
 hw/loongarch/virt.c | 47 ++---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index efce112310..ae79b49774 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -343,6 +343,48 @@ static void fdt_add_memory_node(MachineState *ms,
 g_free(nodename);
 }
 
+static void fdt_add_memory_nodes(MachineState *ms)
+{
+hwaddr base, size, ram_size, gap;
+int i, nb_numa_nodes, nodes;
+NodeInfo *numa_info;
+
+ram_size = ms->ram_size;
+base = VIRT_LOWMEM_BASE;
+gap = VIRT_LOWMEM_SIZE;
+nodes = nb_numa_nodes = ms->numa_state->num_nodes;
+numa_info = ms->numa_state->nodes;
+if (!nodes) {
+nodes = 1;
+}
+
+for (i = 0; i < nodes; i++) {
+if (nb_numa_nodes) {
+size = numa_info[i].node_mem;
+} else {
+size = ram_size;
+}
+
+/*
+ * memory for the node splited into two part
+ *   lowram:  [base, +gap)
+ *   highram: [VIRT_HIGHMEM_BASE, +(len - gap))
+ */
+if (size >= gap) {
+fdt_add_memory_node(ms, base, gap, i);
+size -= gap;
+base = VIRT_HIGHMEM_BASE;
+gap = ram_size - VIRT_LOWMEM_SIZE;
+}
+
+if (size) {
+fdt_add_memory_node(ms, base, size, i);
+base += size;
+gap -= size;
+}
+}
+}
+
 static void virt_build_smbios(LoongArchMachineState *lams)
 {
 MachineState *ms = MACHINE(lams);
@@ -864,9 +906,10 @@ static void loongarch_init(MachineState *machine)
 }
 fdt_add_cpu_nodes(lams);
 
+fdt_add_memory_nodes(machine);
+
 /* Node0 memory */
 memmap_add_entry(VIRT_LOWMEM_BASE, VIRT_LOWMEM_SIZE, 1);
-fdt_add_memory_node(machine, VIRT_LOWMEM_BASE, VIRT_LOWMEM_SIZE, 0);
 memory_region_init_alias(>lowmem, NULL, "loongarch.node0.lowram",
  machine->ram, offset, VIRT_LOWMEM_SIZE);
 memory_region_add_subregion(address_space_mem, phyAddr, >lowmem);
@@ -880,7 +923,6 @@ static void loongarch_init(MachineState *machine)
 }
 phyAddr = VIRT_HIGHMEM_BASE;
 memmap_add_entry(phyAddr, highram_size, 1);
-fdt_add_memory_node(machine, phyAddr, highram_size, 0);
 memory_region_init_alias(>highmem, NULL, "loongarch.node0.highram",
   machine->ram, offset, highram_size);
 memory_region_add_subregion(address_space_mem, phyAddr, >highmem);
@@ -896,7 +938,6 @@ static void loongarch_init(MachineState *machine)
  offset,  numa_info[i].node_mem);
 memory_region_add_subregion(address_space_mem, phyAddr, nodemem);
 memmap_add_entry(phyAddr, numa_info[i].node_mem, 1);
-fdt_add_memory_node(machine, phyAddr, numa_info[i].node_mem, i);
 offset += numa_info[i].node_mem;
 phyAddr += numa_info[i].node_mem;
 }
-- 
2.39.3

Re: [PATCH-for-9.1 03/21] target/ppc: Declare CPU QOM types using DEFINE_TYPES() macro

[Zhao Liu]

On Fri, Mar 15, 2024 at 02:08:51PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:51 +0100
> From: Philippe Mathieu-Daudé 
> Subject: [PATCH-for-9.1 03/21] target/ppc: Declare CPU QOM types using
>  DEFINE_TYPES() macro
> X-Mailer: git-send-email 2.41.0
> 
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. In
> particular because type array declared with such macro
> are easier to review.
> 
> In few commits we are going to add more types, so replace
> the type_register_static() to ease further reviews.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Richard Henderson 
> Message-Id: <20231013140116.255-16-phi...@linaro.org>
> ---
>  target/ppc/cpu_init.c | 52 +++
>  1 file changed, 23 insertions(+), 29 deletions(-)

Reviewed-by: Zhao Liu 

Re: [QEMU PATCH v6 1/1] virtio-pci: implement No_Soft_Reset bit

[Michael S. Tsirkin]

On Thu, Feb 22, 2024 at 11:40:10AM +0800, Jiqian Chen wrote:
> In current code, when guest does S3, virtio devices are reset due to
> the bit No_Soft_Reset is not set. After resetting, the display resources
> of virtio-gpu are destroyed, then the display can't come back and only
> show blank after resuming.
> 
> Implement No_Soft_Reset bit of PCI_PM_CTRL register, then guest can check
> this bit, if this bit is set, the devices resetting will not be done, and
> then the display can work after resuming.
> 
> Signed-off-by: Jiqian Chen 
> ---
>  hw/virtio/virtio-pci.c | 37 +-
>  include/hw/virtio/virtio-pci.h |  5 +
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 1a7039fb0c68..da5312010345 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -2197,6 +2197,11 @@ static void virtio_pci_realize(PCIDevice *pci_dev, 
> Error **errp)
>  pcie_cap_lnkctl_init(pci_dev);
>  }
>  
> +if (proxy->flags & VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET) {
> +pci_set_word(pci_dev->config + pos + PCI_PM_CTRL,
> + PCI_PM_CTRL_NO_SOFT_RESET);
> +}
> +
>  if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
>  /* Init Power Management Control Register */
>  pci_set_word(pci_dev->wmask + pos + PCI_PM_CTRL,


Don't we need compat machinery to avoid breaking migration for
existing machine types?

> @@ -2259,18 +2264,46 @@ static void virtio_pci_reset(DeviceState *qdev)
>  }
>  }
>  
> +static bool device_no_need_reset(PCIDevice *dev)
> +{
> +if (pci_is_express(dev)) {
> +uint16_t pmcsr;
> +
> +pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
> +/*
> + * When No_Soft_Reset bit is set and device

the device

> + * is in D3hot state, can't reset device

can't? or don't?

> + */
> +if ((pmcsr & PCI_PM_CTRL_NO_SOFT_RESET) &&
> +(pmcsr & PCI_PM_CTRL_STATE_MASK) == 3)
> +return true;

coding style violation

> +}
> +
> +return false;
> +}
> +
>  static void virtio_pci_bus_reset_hold(Object *obj)
>  {
>  PCIDevice *dev = PCI_DEVICE(obj);
>  DeviceState *qdev = DEVICE(obj);
>  
> +if (device_no_need_reset(dev))
> +return;
> +
>  virtio_pci_reset(qdev);
>  
>  if (pci_is_express(dev)) {
> +uint16_t pmcsr;
> +uint16_t val = 0;
> +
>  pcie_cap_deverr_reset(dev);
>  pcie_cap_lnkctl_reset(dev);
>  
> -pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, 0);
> +/* don't reset the RO bits */
> +pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
> +val = val | (pmcsr & PCI_PM_CTRL_NO_SOFT_RESET) |
> +(pmcsr & PCI_PM_CTRL_DATA_SCALE_MASK);
> +pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, val);

First, we have test and clear for this.

Second, this has to be conditional on the flag, no?
Without the flag don't we reset everything?
Or you can reuse wmask for this, also an option.

Also what's going on with PCI_PM_CTRL_DATA_SCALE_MASK?
Where does that come from?

>  }
>  }
>  
> @@ -2297,6 +2330,8 @@ static Property virtio_pci_properties[] = {
>  VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, true),
>  DEFINE_PROP_BIT("x-pcie-pm-init", VirtIOPCIProxy, flags,
>  VIRTIO_PCI_FLAG_INIT_PM_BIT, true),
> +DEFINE_PROP_BIT("x-pcie-pm-no-soft-reset", VirtIOPCIProxy, flags,
> +VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT, true),
>  DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags,
>  VIRTIO_PCI_FLAG_INIT_FLR_BIT, true),
>  DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags,
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index 59d88018c16a..9e67ba38c748 100644
> --- a/include/hw/virtio/virtio-pci.h
> +++ b/include/hw/virtio/virtio-pci.h
> @@ -43,6 +43,7 @@ enum {
>  VIRTIO_PCI_FLAG_INIT_FLR_BIT,
>  VIRTIO_PCI_FLAG_AER_BIT,
>  VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED_BIT,
> +VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT,
>  };
>  
>  /* Need to activate work-arounds for buggy guests at vmstate load. */
> @@ -79,6 +80,10 @@ enum {
>  /* Init Power Management */
>  #define VIRTIO_PCI_FLAG_INIT_PM (1 << VIRTIO_PCI_FLAG_INIT_PM_BIT)
>  
> +/* Init The No_Soft_Reset bit of Power Management */
> +#define VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET \
> +  (1 << VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT)
> +
>  /* Init Function Level Reset capability */
>  #define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT)
>  
> -- 
> 2.34.1

[PATCH 4/4] ui/curses: Do not use console_select()

[Akihiko Odaki]

ui/curses is the only user of console_select(). Move the implementation
to ui/curses.

Signed-off-by: Akihiko Odaki 
---
 include/ui/console.h  |   1 -
 ui/console-priv.h |   2 +-
 ui/console-vc-stubs.c |   2 +-
 ui/console-vc.c   |   3 +-
 ui/console.c  | 121 +-
 ui/curses.c   |  48 +++-
 6 files changed, 51 insertions(+), 126 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index a703f7466499..3769179e3b2b 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -433,7 +433,6 @@ int qemu_console_get_window_id(QemuConsole *con);
 /* Set the low-level window id for the console */
 void qemu_console_set_window_id(QemuConsole *con, int window_id);
 
-void console_select(unsigned int index);
 void qemu_console_resize(QemuConsole *con, int width, int height);
 DisplaySurface *qemu_console_surface(QemuConsole *con);
 void coroutine_fn qemu_console_co_wait_update(QemuConsole *con);
diff --git a/ui/console-priv.h b/ui/console-priv.h
index 88569ed2cc41..43ceb8122f13 100644
--- a/ui/console-priv.h
+++ b/ui/console-priv.h
@@ -35,7 +35,7 @@ struct QemuConsole {
 QTAILQ_ENTRY(QemuConsole) next;
 };
 
-void qemu_text_console_select(QemuTextConsole *c);
+void qemu_text_console_update_size(QemuTextConsole *c);
 const char * qemu_text_console_get_label(QemuTextConsole *c);
 void qemu_text_console_update_cursor(void);
 void qemu_text_console_handle_keysym(QemuTextConsole *s, int keysym);
diff --git a/ui/console-vc-stubs.c b/ui/console-vc-stubs.c
index 2afc52329f0c..b63e2fb2345f 100644
--- a/ui/console-vc-stubs.c
+++ b/ui/console-vc-stubs.c
@@ -10,7 +10,7 @@
 #include "chardev/char.h"
 #include "ui/console-priv.h"
 
-void qemu_text_console_select(QemuTextConsole *c)
+void qemu_text_console_update_size(QemuTextConsole *c)
 {
 }
 
diff --git a/ui/console-vc.c b/ui/console-vc.c
index f22c8e23c2ed..899fa11c9485 100644
--- a/ui/console-vc.c
+++ b/ui/console-vc.c
@@ -958,10 +958,9 @@ static void vc_chr_set_echo(Chardev *chr, bool echo)
 drv->console->echo = echo;
 }
 
-void qemu_text_console_select(QemuTextConsole *c)
+void qemu_text_console_update_size(QemuTextConsole *c)
 {
 dpy_text_resize(QEMU_CONSOLE(c), c->width, c->height);
-qemu_text_console_update_cursor();
 }
 
 static void vc_chr_open(Chardev *chr,
diff --git a/ui/console.c b/ui/console.c
index 6bf02a23414c..d77f509046af 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -66,7 +66,6 @@ struct DisplayState {
 };
 
 static DisplayState *display_state;
-static QemuConsole *active_console;
 static QTAILQ_HEAD(, QemuConsole) consoles =
 QTAILQ_HEAD_INITIALIZER(consoles);
 
@@ -135,7 +134,6 @@ void graphic_hw_update_done(QemuConsole *con)
 void graphic_hw_update(QemuConsole *con)
 {
 bool async = false;
-con = con ? con : active_console;
 if (!con) {
 return;
 }
@@ -209,9 +207,6 @@ void qemu_console_set_window_id(QemuConsole *con, int 
window_id)
 
 void graphic_hw_invalidate(QemuConsole *con)
 {
-if (!con) {
-con = active_console;
-}
 if (con && con->hw_ops->invalidate) {
 con->hw_ops->invalidate(con->hw);
 }
@@ -219,9 +214,6 @@ void graphic_hw_invalidate(QemuConsole *con)
 
 void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata)
 {
-if (!con) {
-con = active_console;
-}
 if (con && con->hw_ops->text_update) {
 con->hw_ops->text_update(con->hw, chardata);
 }
@@ -265,12 +257,12 @@ static void dpy_gfx_update_texture(QemuConsole *con, 
DisplaySurface *surface,
 }
 
 static void displaychangelistener_display_console(DisplayChangeListener *dcl,
-  QemuConsole *con,
   Error **errp)
 {
 static const char nodev[] =
 "This VM has no graphic display device.";
 static DisplaySurface *dummy;
+QemuConsole *con = dcl->con;
 
 if (!con || !console_compatible_with(con, dcl, errp)) {
 if (!dummy) {
@@ -305,39 +297,8 @@ static void 
displaychangelistener_display_console(DisplayChangeListener *dcl,
 }
 }
 
-void console_select(unsigned int index)
-{
-DisplayChangeListener *dcl;
-QemuConsole *s;
-
-trace_console_select(index);
-s = qemu_console_lookup_by_index(index);
-if (s) {
-DisplayState *ds = s->ds;
-
-active_console = s;
-QLIST_FOREACH (dcl, >listeners, next) {
-if (dcl->con != NULL) {
-continue;
-}
-displaychangelistener_display_console(dcl, s, NULL);
-}
-
-if (QEMU_IS_TEXT_CONSOLE(s)) {
-qemu_text_console_select(QEMU_TEXT_CONSOLE(s));
-}
-}
-}
-
 void qemu_text_console_put_keysym(QemuTextConsole *s, int keysym)
 {
-if (!s) {
-if (!QEMU_IS_TEXT_CONSOLE(active_console)) {
-return;
-}
-s = QEMU_TEXT_CONSOLE(active_console);
-}
-
 

[PATCH 3/4] ui/cocoa: Do not use console_select()

[Akihiko Odaki]

ui/cocoa needs to update the UI info and reset the keyboard state
tracker when switching the console, or the new console will see the
stale UI info or keyboard state. Previously, updating the UI info was
done with cocoa_switch(), but it is meant to be called when the surface
is being replaced, and may be called even when not switching the
console. ui/cocoa never reset the keyboard state, which resulted in
stuck keys.

Add ui/cocoa's own implementation of console_select(), which updates the
UI info and resets the keyboard state tracker.

Signed-off-by: Akihiko Odaki 
---
 ui/cocoa.m | 37 ++---
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index fa879d7dcd4b..47280c0a93be 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -102,6 +102,7 @@ static void cocoa_switch(DisplayChangeListener *dcl,
 static DisplayChangeListener dcl = {
 .ops = _ops,
 };
+static QKbdState *kbd;
 static int cursor_hide = 1;
 static int left_command_key_enabled = 1;
 static bool swap_opt_cmd;
@@ -309,7 +310,6 @@ @interface QemuCocoaView : NSView
 NSTrackingArea *trackingArea;
 QEMUScreen screen;
 pixman_image_t *pixman_image;
-QKbdState *kbd;
 BOOL isMouseGrabbed;
 BOOL isAbsoluteEnabled;
 CFMachPortRef eventsTap;
@@ -361,7 +361,6 @@ - (id)initWithFrame:(NSRect)frameRect
 
 screen.width = frameRect.size.width;
 screen.height = frameRect.size.height;
-kbd = qkbd_state_init(dcl.con);
 #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_14_0
 [self setClipsToBounds:YES];
 #endif
@@ -378,8 +377,6 @@ - (void) dealloc
 pixman_image_unref(pixman_image);
 }
 
-qkbd_state_free(kbd);
-
 if (eventsTap) {
 CFRelease(eventsTap);
 }
@@ -429,6 +426,20 @@ - (void) viewWillMoveToWindow:(NSWindow *)newWindow
 [self removeTrackingRect];
 }
 
+- (void) selectConsoleLocked:(unsigned int)index
+{
+QemuConsole *con = qemu_console_lookup_by_index(index);
+if (!con) {
+return;
+}
+
+unregister_displaychangelistener();
+qkbd_state_switch_console(kbd, con);
+dcl.con = con;
+register_displaychangelistener();
+[self updateUIInfo];
+}
+
 - (void) hideCursor
 {
 if (!cursor_hide) {
@@ -718,7 +729,8 @@ - (void) handleMonitorInput:(NSEvent *)event
 }
 
 if (keysym) {
-qemu_text_console_put_keysym(NULL, keysym);
+QemuTextConsole *con = QEMU_TEXT_CONSOLE(dcl.con);
+qemu_text_console_put_keysym(con, keysym);
 }
 }
 
@@ -898,7 +910,7 @@ - (bool) handleEventLocked:(NSEvent *)event
 
 // enable graphic console
 case '1' ... '9':
-console_select(key - '0' - 1); /* ascii math */
+[self selectConsoleLocked:key - '0' - 1]; /* ascii 
math */
 return true;
 
 // release the mouse grab
@@ -909,7 +921,7 @@ - (bool) handleEventLocked:(NSEvent *)event
 }
 }
 
-if (qemu_console_is_graphic(NULL)) {
+if (qemu_console_is_graphic(dcl.con)) {
 qkbd_state_key_event(kbd, keycode, true);
 } else {
 [self handleMonitorInput: event];
@@ -924,7 +936,7 @@ - (bool) handleEventLocked:(NSEvent *)event
 return true;
 }
 
-if (qemu_console_is_graphic(NULL)) {
+if (qemu_console_is_graphic(dcl.con)) {
 qkbd_state_key_event(kbd, keycode, false);
 }
 return true;
@@ -1374,7 +1386,7 @@ - (void)toggleZoomInterpolation:(id) sender
 - (void)displayConsole:(id)sender
 {
 with_bql(^{
-console_select([sender tag]);
+[cocoaView selectConsoleLocked:[sender tag]];
 });
 }
 
@@ -1945,7 +1957,6 @@ static void cocoa_switch(DisplayChangeListener *dcl,
 pixman_image_ref(image);
 
 dispatch_async(dispatch_get_main_queue(), ^{
-[cocoaView updateUIInfo];
 [cocoaView switchSurface:image];
 });
 }
@@ -1955,7 +1966,7 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
 NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
 
 COCOA_DEBUG("qemu_cocoa: cocoa_refresh\n");
-graphic_hw_update(NULL);
+graphic_hw_update(dcl->con);
 
 if (qemu_input_is_absolute(dcl->con)) {
 dispatch_async(dispatch_get_main_queue(), ^{
@@ -2039,8 +2050,12 @@ static void cocoa_display_init(DisplayState *ds, 
DisplayOptions *opts)
 add_console_menu_entries();
 addRemovableDevicesMenuItems();
 
+dcl.con = qemu_console_lookup_first_graphic_console();
+kbd = qkbd_state_init(dcl.con);
+
 // register vga output callbacks
 register_displaychangelistener();
+[cocoaView updateUIInfo];
 
 qemu_event_init(, false);
 cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];

-- 
2.44.0

[PATCH 1/4] ui/vc: Do not inherit the size of active console

[Akihiko Odaki]

A chardev-vc used to inherit the size of a graphic console when its
size not explicitly specified, but it often did not make sense. If a
chardev-vc is instantiated during the startup, the active graphic
console has no content at the time, so it will have the size of graphic
console placeholder, which contains no useful information. It's better
to have the standard size of text console instead.

Signed-off-by: Akihiko Odaki 
---
 ui/console-vc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/console-vc.c b/ui/console-vc.c
index 9c13cc2981b0..f22c8e23c2ed 100644
--- a/ui/console-vc.c
+++ b/ui/console-vc.c
@@ -990,8 +990,8 @@ static void vc_chr_open(Chardev *chr,
 trace_console_txt_new(width, height);
 if (width == 0 || height == 0) {
 s = QEMU_TEXT_CONSOLE(object_new(TYPE_QEMU_TEXT_CONSOLE));
-width = qemu_console_get_width(NULL, 80 * FONT_WIDTH);
-height = qemu_console_get_height(NULL, 24 * FONT_HEIGHT);
+width = 80 * FONT_WIDTH;
+height = 24 * FONT_HEIGHT;
 } else {
 s = QEMU_TEXT_CONSOLE(object_new(TYPE_QEMU_FIXED_TEXT_CONSOLE));
 }

-- 
2.44.0

[PATCH 1/2] ui/cocoa: Add cursor composition

[Akihiko Odaki]

Add accelerated cursor composition to ui/cocoa. This does not only
improve performance for display devices that exposes the capability to
the guest according to dpy_cursor_define_supported(), but fixes the
cursor display for devices that unconditionally expects the availability
of the capability (e.g., virtio-gpu).

The common pattern to implement accelerated cursor composition is to
replace the cursor and warp it so that the replaced cursor is shown at
the correct position on the guest display. Unfortunately, ui/cocoa
cannot do the same because warping the cursor position interfers with
the mouse input so it uses CALayer instead; although it is not
specialized for cursor composition, it still can compose images with
hardware acceleration.

Signed-off-by: Akihiko Odaki 
---
 meson.build |  3 +-
 ui/cocoa.m  | 97 +
 2 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index b375248a7614..b198ca2972ed 100644
--- a/meson.build
+++ b/meson.build
@@ -1044,7 +1044,8 @@ if get_option('attr').allowed()
   endif
 endif
 
-cocoa = dependency('appleframeworks', modules: ['Cocoa', 'CoreVideo'],
+cocoa = dependency('appleframeworks',
+   modules: ['Cocoa', 'CoreVideo', 'QuartzCore'],
required: get_option('cocoa'))
 
 vmnet = dependency('appleframeworks', modules: 'vmnet', required: 
get_option('vmnet'))
diff --git a/ui/cocoa.m b/ui/cocoa.m
index fa879d7dcd4b..c13c2a01e947 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 
 #import 
+#import 
 #include 
 
 #include "qemu/help-texts.h"
@@ -92,12 +93,16 @@ static void cocoa_switch(DisplayChangeListener *dcl,
  DisplaySurface *surface);
 
 static void cocoa_refresh(DisplayChangeListener *dcl);
+static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int on);
+static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor 
*cursor);
 
 static const DisplayChangeListenerOps dcl_ops = {
 .dpy_name  = "cocoa",
 .dpy_gfx_update = cocoa_update,
 .dpy_gfx_switch = cocoa_switch,
 .dpy_refresh = cocoa_refresh,
+.dpy_mouse_set = cocoa_mouse_set,
+.dpy_cursor_define = cocoa_cursor_define,
 };
 static DisplayChangeListener dcl = {
 .ops = _ops,
@@ -313,6 +318,11 @@ @interface QemuCocoaView : NSView
 BOOL isMouseGrabbed;
 BOOL isAbsoluteEnabled;
 CFMachPortRef eventsTap;
+CALayer *cursorLayer;
+QEMUCursor *cursor;
+int mouseX;
+int mouseY;
+int mouseOn;
 }
 - (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
@@ -365,6 +375,12 @@ - (id)initWithFrame:(NSRect)frameRect
 #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_14_0
 [self setClipsToBounds:YES];
 #endif
+[self setWantsLayer:YES];
+cursorLayer = [[CALayer alloc] init];
+[cursorLayer setAnchorPoint:CGPointMake(0, 1)];
+[cursorLayer setAutoresizingMask:kCALayerMaxXMargin |
+ kCALayerMinYMargin];
+[[self layer] addSublayer:cursorLayer];
 
 }
 return self;
@@ -445,6 +461,72 @@ - (void) unhideCursor
 [NSCursor unhide];
 }
 
+- (void)setMouseX:(int)x y:(int)y on:(int)on
+{
+CGPoint position;
+
+mouseX = x;
+mouseY = y;
+mouseOn = on;
+
+position.x = mouseX;
+position.y = screen.height - mouseY;
+
+[CATransaction begin];
+[CATransaction setDisableActions:YES];
+[cursorLayer setPosition:position];
+[cursorLayer setHidden:!mouseOn];
+[CATransaction commit];
+}
+
+- (void)setCursor:(QEMUCursor *)given_cursor
+{
+CGDataProviderRef provider;
+CGImageRef image;
+CGRect bounds = CGRectZero;
+
+cursor_unref(cursor);
+cursor = given_cursor;
+
+if (!cursor) {
+return;
+}
+
+cursor_ref(cursor);
+
+bounds.size.width = cursor->width;
+bounds.size.height = cursor->height;
+
+provider = CGDataProviderCreateWithData(
+NULL,
+cursor->data,
+cursor->width * cursor->height * 4,
+NULL
+);
+
+image = CGImageCreate(
+cursor->width, //width
+cursor->height, //height
+8, //bitsPerComponent
+32, //bitsPerPixel
+cursor->width * 4, //bytesPerRow
+CGColorSpaceCreateWithName(kCGColorSpaceSRGB), //colorspace
+kCGBitmapByteOrder32Little | kCGImageAlphaFirst, //bitmapInfo
+provider, //provider
+NULL, //decode
+0, //interpolate
+kCGRenderingIntentDefault //intent
+);
+
+CGDataProviderRelease(provider);
+[CATransaction begin];
+[CATransaction setDisableActions:YES];
+[cursorLayer setBounds:bounds];
+[cursorLayer setContents:(id)image];
+[CATransaction commit];
+CGImageRelease(image);
+}
+
 - (void) drawRect:(NSRect) rect
 {
 COCOA_DEBUG("QemuCocoaView: drawRect\n");
@@ -1982,6 +2064,21 @@ static void 

[PATCH 2/4] ui/vnc: Do not use console_select()

[Akihiko Odaki]

console_select() is shared by other displays and a console_select() call
from one of them triggers console switching also in ui/curses,
circumventing key state reinitialization that needs to be performed in
preparation and resulting in stuck keys.

Use its internal state to track the current active console to prevent
such a surprise console switch.

Signed-off-by: Akihiko Odaki 
---
 include/ui/console.h   |  1 +
 include/ui/kbd-state.h | 11 +++
 ui/console.c   | 12 
 ui/kbd-state.c |  6 ++
 ui/vnc.c   | 14 +-
 5 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index a4a49ffc640c..a703f7466499 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -413,6 +413,7 @@ void qemu_console_early_init(void);
 
 void qemu_console_set_display_gl_ctx(QemuConsole *con, DisplayGLCtx *ctx);
 
+QemuConsole *qemu_console_lookup_first_graphic_console(void);
 QemuConsole *qemu_console_lookup_by_index(unsigned int index);
 QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head);
 QemuConsole *qemu_console_lookup_by_device_name(const char *device_id,
diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
index fb79776128cf..1f37b932eb62 100644
--- a/include/ui/kbd-state.h
+++ b/include/ui/kbd-state.h
@@ -99,4 +99,15 @@ bool qkbd_state_modifier_get(QKbdState *kbd, QKbdModifier 
mod);
  */
 void qkbd_state_lift_all_keys(QKbdState *kbd);
 
+/**
+ * qkbd_state_switch_console: Switch console.
+ *
+ * This sends key up events to the previous console for all keys which are in
+ * down state to prevent keys being stuck, and remembers the new console.
+ *
+ * @kbd: state tracker state.
+ * @con: new QemuConsole for this state tracker.
+ */
+void qkbd_state_switch_console(QKbdState *kbd, QemuConsole *con);
+
 #endif /* QEMU_UI_KBD_STATE_H */
diff --git a/ui/console.c b/ui/console.c
index 832055675c50..6bf02a23414c 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1325,6 +1325,18 @@ void graphic_console_close(QemuConsole *con)
 dpy_gfx_replace_surface(con, surface);
 }
 
+QemuConsole *qemu_console_lookup_first_graphic_console(void)
+{
+QemuConsole *con;
+
+QTAILQ_FOREACH(con, , next) {
+if (QEMU_IS_GRAPHIC_CONSOLE(con)) {
+return con;
+}
+}
+return NULL;
+}
+
 QemuConsole *qemu_console_lookup_by_index(unsigned int index)
 {
 QemuConsole *con;
diff --git a/ui/kbd-state.c b/ui/kbd-state.c
index 62d42a7a22e1..52ed28b8a89b 100644
--- a/ui/kbd-state.c
+++ b/ui/kbd-state.c
@@ -117,6 +117,12 @@ void qkbd_state_lift_all_keys(QKbdState *kbd)
 }
 }
 
+void qkbd_state_switch_console(QKbdState *kbd, QemuConsole *con)
+{
+qkbd_state_lift_all_keys(kbd);
+kbd->con = con;
+}
+
 void qkbd_state_set_delay(QKbdState *kbd, int delay_ms)
 {
 kbd->key_delay_ms = delay_ms;
diff --git a/ui/vnc.c b/ui/vnc.c
index fc12b343e254..94564b196ba8 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1872,12 +1872,16 @@ static void do_key_event(VncState *vs, int down, int 
keycode, int sym)
 /* QEMU console switch */
 switch (qcode) {
 case Q_KEY_CODE_1 ... Q_KEY_CODE_9: /* '1' to '9' keys */
-if (vs->vd->dcl.con == NULL && down &&
+if (down &&
 qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_CTRL) &&
 qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_ALT)) {
-/* Reset the modifiers sent to the current console */
-qkbd_state_lift_all_keys(vs->vd->kbd);
-console_select(qcode - Q_KEY_CODE_1);
+QemuConsole *con = qemu_console_lookup_by_index(qcode - 
Q_KEY_CODE_1);
+if (con) {
+unregister_displaychangelistener(>vd->dcl);
+qkbd_state_switch_console(vs->vd->kbd, con);
+vs->vd->dcl.con = con;
+register_displaychangelistener(>vd->dcl);
+}
 return;
 }
 default:
@@ -4206,7 +4210,7 @@ void vnc_display_open(const char *id, Error **errp)
 goto fail;
 }
 } else {
-con = NULL;
+con = qemu_console_lookup_first_graphic_console();
 }
 
 if (con != vd->dcl.con) {

-- 
2.44.0

[PATCH 0/4] ui/console: Remove console_select()

[Akihiko Odaki]

ui/console has a concept of "active" console; the active console is used
when NULL is set for DisplayListener::con, and console_select() updates
the active console state. However, the global nature of the state cause
odd behaviors, and replacing NULL with the active console also resulted
in extra code. Remove it to solve these problems.

The active console state is shared, so if there are two displays
referring to the active console, switching the console for one will also
affect the other. All displays that use the active console state,
namely cocoa, curses, and vnc, need to reset some of its state before
switching the console, and such a reset operation cannot be performed if
the console is switched by another display. This can result in stuck
keys, for example.

While the active console state is shared, displays other than cocoa,
curses, and vnc don't update the state. A chardev-vc inherits the
size of the active console, but it does not make sense for such a
display.

This series removes the shared "active" console state from ui/console.
curses, cocoa, and vnc will hold the reference to the console currently
shown with DisplayListener::con. This also eliminates the need to
replace NULL with the active console and save code.

Signed-off-by: Akihiko Odaki 
---
Akihiko Odaki (4):
  ui/vc: Do not inherit the size of active console
  ui/vnc: Do not use console_select()
  ui/cocoa: Do not use console_select()
  ui/curses: Do not use console_select()

 include/ui/console.h   |   2 +-
 include/ui/kbd-state.h |  11 
 ui/console-priv.h  |   2 +-
 ui/console-vc-stubs.c  |   2 +-
 ui/console-vc.c|   7 ++-
 ui/console.c   | 133 -
 ui/curses.c|  48 ++
 ui/kbd-state.c |   6 +++
 ui/vnc.c   |  14 --
 ui/cocoa.m |  37 ++
 10 files changed, 118 insertions(+), 144 deletions(-)
---
base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
change-id: 20240317-console-6744d4ab8ba6

Best regards,
-- 
Akihiko Odaki 

Re: [PATCH] vhost-vdpa: check vhost_vdpa_set_vring_ready() return value

[Stefano Garzarella]

On Mon, Mar 18, 2024 at 12:31:59PM +0800, Jason Wang wrote:

On Fri, Mar 15, 2024 at 4:23 PM Stefano Garzarella  wrote:


On Thu, Mar 14, 2024 at 11:17:01AM +0800, Jason Wang wrote:
>On Wed, Feb 7, 2024 at 5:27 PM Stefano Garzarella  wrote:
>>
>> vhost_vdpa_set_vring_ready() could already fail, but if Linux's
>> patch [1] will be merged, it may fail with more chance if
>> userspace does not activate virtqueues before DRIVER_OK when
>> VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK is not negotiated.
>
>I wonder what happens if we just leave it as is.

Are you referring to this patch or the kernel patch?


This patch.



Here I'm just checking the return value of vhost_vdpa_set_vring_ready().
It can return an error also without that kernel patch, so IMHO is
better to check the return value here in QEMU.

What issue do you see with this patch applied?


For the parent which can enable after driver_ok but not advertise it.


But this patch is not changing anything in that sense, it just controls
the return value of the VHOST_VDPA_SET_VRING_ENABLE ioctl.

Why would QEMU ignore an error if it can't activate vrings?
If we really want to ignore it we should document it both in QEMU, but
also in the kernel, because honestly the way the code is now it
shouldn't fail from what I understand.

That said, even if we ignore it, IMHO we should at least print a warning
in QEMU.



(To say the truth, I'm not sure if we need to care about this)


I agree on that, but this is related to the patch in the kernel, not
this simple patch to fix QEMU error path, right?





>
>VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK: We do know enabling could be
>done after driver_ok.
>Without VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK: We don't know whether
>enabling could be done after driver_ok or not.

I see your point, indeed I didn't send a v2 of that patch.
Maybe we should document that, because it could be interpreted that if
VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK is not negotiated the enabling
should always be done before driver_ok (which is true for example in
VDUSE).


I see, so I think we probably need the fix.



BTW I think we should discuss it in the kernel patch.

Thanks,
Stefano

>
>Thanks
>
>>
>> So better check its return value anyway.
>>
>> [1] 
https://lore.kernel.org/virtualization/20240206145154.118044-1-sgarz...@redhat.com/T/#u
>>
>> Signed-off-by: Stefano Garzarella 
>> ---
>> Note: This patch conflicts with [2], but the resolution is simple,
>> so for now I sent a patch for the current master, but I'll rebase
>> this patch if we merge the other one first.


Will go through [2].


Here I meant that the conflict is only in the code touched, because
Kevin's patch remove/move some of the code touched by this patch.
And rightly he checked the return value of the ioctl as I would like to
do in the other places where we call the same ioctl.

So honestly I still don't understand what's wrong with this patch...

Thanks,
Stefano



Thanks



>>
>> [2]
>> https://lore.kernel.org/qemu-devel/20240202132521.32714-1-kw...@redhat.com/
>> ---
>>  hw/virtio/vdpa-dev.c |  8 +++-
>>  net/vhost-vdpa.c | 15 ---
>>  2 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
>> index eb9ecea83b..d57cd76c18 100644
>> --- a/hw/virtio/vdpa-dev.c
>> +++ b/hw/virtio/vdpa-dev.c
>> @@ -259,7 +259,11 @@ static int vhost_vdpa_device_start(VirtIODevice *vdev, 
Error **errp)
>>  goto err_guest_notifiers;
>>  }
>>  for (i = 0; i < s->dev.nvqs; ++i) {
>> -vhost_vdpa_set_vring_ready(>vdpa, i);
>> +ret = vhost_vdpa_set_vring_ready(>vdpa, i);
>> +if (ret < 0) {
>> +error_setg_errno(errp, -ret, "Error starting vring %d", i);
>> +goto err_dev_stop;
>> +}
>>  }
>>  s->started = true;
>>
>> @@ -274,6 +278,8 @@ static int vhost_vdpa_device_start(VirtIODevice *vdev, 
Error **errp)
>>
>>  return ret;
>>
>> +err_dev_stop:
>> +vhost_dev_stop(>dev, vdev, false);
>>  err_guest_notifiers:
>>  k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
>>  err_host_notifiers:
>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>> index 3726ee5d67..e3d8036479 100644
>> --- a/net/vhost-vdpa.c
>> +++ b/net/vhost-vdpa.c
>> @@ -381,7 +381,10 @@ static int vhost_vdpa_net_data_load(NetClientState *nc)
>>  }
>>
>>  for (int i = 0; i < v->dev->nvqs; ++i) {
>> -vhost_vdpa_set_vring_ready(v, i + v->dev->vq_index);
>> +int ret = vhost_vdpa_set_vring_ready(v, i + v->dev->vq_index);
>> +if (ret < 0) {
>> +return ret;
>> +}
>>  }
>>  return 0;
>>  }
>> @@ -1213,7 +1216,10 @@ static int vhost_vdpa_net_cvq_load(NetClientState *nc)
>>
>>  assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
>>
>> -vhost_vdpa_set_vring_ready(v, v->dev->vq_index);
>> +r = vhost_vdpa_set_vring_ready(v, v->dev->vq_index);
>> +if (unlikely(r < 0)) {
>> +return r;
>> +}
>>

[PATCH V6] target/loongarch: Fix tlb huge page loading issue

[Xianglai Li]

When we use qemu tcg simulation, the page size of bios is 4KB.
When using the level 2 super huge page (page size is 1G) to create the page 
table,
it is found that the content of the corresponding address space is abnormal,
resulting in the bios can not start the operating system and graphical 
interface normally.

The lddir and ldpte instruction emulation has
a problem with the use of super huge page processing above level 2.
The page size is not correctly calculated,
resulting in the wrong page size of the table entry found by tlb.

Signed-off-by: Xianglai Li 
---
 target/loongarch/cpu-csr.h|   3 +
 target/loongarch/internals.h  |   5 --
 target/loongarch/tcg/tlb_helper.c | 113 +-
 3 files changed, 82 insertions(+), 39 deletions(-)

Changes log:
V5->V6:
Add some necessary log printing to patch.

V4->V5:
Modifying the patch Title.
Fix incorrect usage of FIELD macro and code logic errors in patch.

V3->V4:
Optimize the huge page calculation method,
use the FIELD macro for bit calculation.

V2->V3:
Delete the intermediate variable LDDIR_PS, and implement lddir and ldpte
huge pages by referring to the latest architecture reference manual.

V1->V2:
Modified the patch title format and Enrich the commit mesg description

diff --git a/target/loongarch/cpu-csr.h b/target/loongarch/cpu-csr.h
index c59d7a9fcb..0834e91f30 100644
--- a/target/loongarch/cpu-csr.h
+++ b/target/loongarch/cpu-csr.h
@@ -67,6 +67,9 @@ FIELD(TLBENTRY, D, 1, 1)
 FIELD(TLBENTRY, PLV, 2, 2)
 FIELD(TLBENTRY, MAT, 4, 2)
 FIELD(TLBENTRY, G, 6, 1)
+FIELD(TLBENTRY, HUGE, 6, 1)
+FIELD(TLBENTRY, HGLOBAL, 12, 1)
+FIELD(TLBENTRY, LEVEL, 13, 2)
 FIELD(TLBENTRY_32, PPN, 8, 24)
 FIELD(TLBENTRY_64, PPN, 12, 36)
 FIELD(TLBENTRY_64, NR, 61, 1)
diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
index a2fc54c8a7..944153b180 100644
--- a/target/loongarch/internals.h
+++ b/target/loongarch/internals.h
@@ -16,11 +16,6 @@
 #define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)
 #define TARGET_VIRT_MASK MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS)
 
-/* Global bit used for lddir/ldpte */
-#define LOONGARCH_PAGE_HUGE_SHIFT   6
-/* Global bit for huge page */
-#define LOONGARCH_HGLOBAL_SHIFT 12
-
 void loongarch_translate_init(void);
 
 void loongarch_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
diff --git a/target/loongarch/tcg/tlb_helper.c 
b/target/loongarch/tcg/tlb_helper.c
index 22be031ac7..57f5308632 100644
--- a/target/loongarch/tcg/tlb_helper.c
+++ b/target/loongarch/tcg/tlb_helper.c
@@ -17,6 +17,34 @@
 #include "exec/log.h"
 #include "cpu-csr.h"
 
+static void get_dir_base_width(CPULoongArchState *env, uint64_t *dir_base,
+   uint64_t *dir_width, target_ulong level)
+{
+switch (level) {
+case 1:
+*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
+break;
+case 2:
+*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
+break;
+case 3:
+*dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
+break;
+case 4:
+*dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
+break;
+default:
+/* level may be zero for ldpte */
+*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
+*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
+break;
+}
+}
+
 static void raise_mmu_exception(CPULoongArchState *env, target_ulong address,
 MMUAccessType access_type, int tlb_error)
 {
@@ -485,7 +513,25 @@ target_ulong helper_lddir(CPULoongArchState *env, 
target_ulong base,
 target_ulong badvaddr, index, phys, ret;
 int shift;
 uint64_t dir_base, dir_width;
-bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
+
+if (unlikely((level == 0) || (level > 4))) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "Attepted LDDIR with level %"PRId64"\n", level);
+return base;
+}
+
+if (FIELD_EX64(base, TLBENTRY, HUGE)) {
+if (unlikely(level == 4)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "Attempted use of level 4 huge page\n");
+}
+
+if (FIELD_EX64(base, TLBENTRY, LEVEL)) {
+return base;
+} else {
+return FIELD_DP64(base, TLBENTRY, LEVEL, level);
+}
+}
 
 badvaddr = env->CSR_TLBRBADV;
 base = base & TARGET_PHYS_MASK;
@@ -494,30 +540,7 @@ target_ulong helper_lddir(CPULoongArchState *env, 
target_ulong base,
 shift = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTEWIDTH);
 shift = (shift + 1) * 3;
 
-if (huge) {
-return base;
-}
-switch 

Re: Regression in v7.2.10 - ui-dbus.so requires -fPIC

[Marc-André Lureau]

Hi

On Sun, Mar 17, 2024 at 11:10 AM Michael Tokarev  wrote:
>
> 17.03.2024 01:19, Olaf Hering:
> > Sat, 16 Mar 2024 22:40:14 +0300 Michael Tokarev :
> >
> >>   meson: ensure dbus-display generated code is built before other units
> >>   (cherry picked from commit 1222070e772833c6875e0ca63565db12c22df39e)
> >
> > "static_library" is used often. Some use the 'pic' option, which fixes the 
> > issue.
> >
> > I think every usage that ends up in a shared library requires 'pic:true'.
>
> The prob here seems to be that while fixing other issue (header file isn't 
> generated
> early enough), we all forgot about the fact dbus-display1.o can be used 
> inside a
> shared/loadable object when qemu is built with --enable-modules.
>

dbus-display1 is also used with static linking for the unit test.

It looks like the simplest is to let the actual target decide how it
is built, even if it is compiled multiple time then:

-  dbus_display1_lib = static_library('dbus-display1', dbus_display1,
dependencies: gio)
-  dbus_display1_dep = declare_dependency(link_with:
dbus_display1_lib, sources: dbus_display1[0])
+  dbus_display1_dep = declare_dependency(sources: dbus_display1,
dependencies: gio)

This makes commit 186acfbaf7 ("tests/qtest: Depend on
dbus_display1_dep") no longer relevant, as dbus-display1.c will be
recompiled.

Alternatively, we could always build with pic: true (or pic:
enable_modules), but that's not ideal either.

-- 
Marc-André Lureau

Re: [QEMU PATCH v6 1/1] virtio-pci: implement No_Soft_Reset bit

[Chen, Jiqian]

Hi Michael S. Tsirkin,
Do you have any comments on this patch?

On 2024/2/22 11:40, Jiqian Chen wrote:
> In current code, when guest does S3, virtio devices are reset due to
> the bit No_Soft_Reset is not set. After resetting, the display resources
> of virtio-gpu are destroyed, then the display can't come back and only
> show blank after resuming.
> 
> Implement No_Soft_Reset bit of PCI_PM_CTRL register, then guest can check
> this bit, if this bit is set, the devices resetting will not be done, and
> then the display can work after resuming.
> 
> Signed-off-by: Jiqian Chen 
> ---
>  hw/virtio/virtio-pci.c | 37 +-
>  include/hw/virtio/virtio-pci.h |  5 +
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 1a7039fb0c68..da5312010345 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -2197,6 +2197,11 @@ static void virtio_pci_realize(PCIDevice *pci_dev, 
> Error **errp)
>  pcie_cap_lnkctl_init(pci_dev);
>  }
>  
> +if (proxy->flags & VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET) {
> +pci_set_word(pci_dev->config + pos + PCI_PM_CTRL,
> + PCI_PM_CTRL_NO_SOFT_RESET);
> +}
> +
>  if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
>  /* Init Power Management Control Register */
>  pci_set_word(pci_dev->wmask + pos + PCI_PM_CTRL,
> @@ -2259,18 +2264,46 @@ static void virtio_pci_reset(DeviceState *qdev)
>  }
>  }
>  
> +static bool device_no_need_reset(PCIDevice *dev)
> +{
> +if (pci_is_express(dev)) {
> +uint16_t pmcsr;
> +
> +pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
> +/*
> + * When No_Soft_Reset bit is set and device
> + * is in D3hot state, can't reset device
> + */
> +if ((pmcsr & PCI_PM_CTRL_NO_SOFT_RESET) &&
> +(pmcsr & PCI_PM_CTRL_STATE_MASK) == 3)
> +return true;
> +}
> +
> +return false;
> +}
> +
>  static void virtio_pci_bus_reset_hold(Object *obj)
>  {
>  PCIDevice *dev = PCI_DEVICE(obj);
>  DeviceState *qdev = DEVICE(obj);
>  
> +if (device_no_need_reset(dev))
> +return;
> +
>  virtio_pci_reset(qdev);
>  
>  if (pci_is_express(dev)) {
> +uint16_t pmcsr;
> +uint16_t val = 0;
> +
>  pcie_cap_deverr_reset(dev);
>  pcie_cap_lnkctl_reset(dev);
>  
> -pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, 0);
> +/* don't reset the RO bits */
> +pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
> +val = val | (pmcsr & PCI_PM_CTRL_NO_SOFT_RESET) |
> +(pmcsr & PCI_PM_CTRL_DATA_SCALE_MASK);
> +pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, val);
>  }
>  }
>  
> @@ -2297,6 +2330,8 @@ static Property virtio_pci_properties[] = {
>  VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, true),
>  DEFINE_PROP_BIT("x-pcie-pm-init", VirtIOPCIProxy, flags,
>  VIRTIO_PCI_FLAG_INIT_PM_BIT, true),
> +DEFINE_PROP_BIT("x-pcie-pm-no-soft-reset", VirtIOPCIProxy, flags,
> +VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT, true),
>  DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags,
>  VIRTIO_PCI_FLAG_INIT_FLR_BIT, true),
>  DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags,
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index 59d88018c16a..9e67ba38c748 100644
> --- a/include/hw/virtio/virtio-pci.h
> +++ b/include/hw/virtio/virtio-pci.h
> @@ -43,6 +43,7 @@ enum {
>  VIRTIO_PCI_FLAG_INIT_FLR_BIT,
>  VIRTIO_PCI_FLAG_AER_BIT,
>  VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED_BIT,
> +VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT,
>  };
>  
>  /* Need to activate work-arounds for buggy guests at vmstate load. */
> @@ -79,6 +80,10 @@ enum {
>  /* Init Power Management */
>  #define VIRTIO_PCI_FLAG_INIT_PM (1 << VIRTIO_PCI_FLAG_INIT_PM_BIT)
>  
> +/* Init The No_Soft_Reset bit of Power Management */
> +#define VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET \
> +  (1 << VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT)
> +
>  /* Init Function Level Reset capability */
>  #define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT)
>  

-- 
Best regards,
Jiqian Chen.

Re: [PATCH-for-9.1 05/21] cpus: Open code OBJECT_DECLARE_TYPE() in OBJECT_DECLARE_CPU_TYPE()

[Zhao Liu]

Hi Philippe,

On Fri, Mar 15, 2024 at 02:08:53PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:53 +0100
> From: Philippe Mathieu-Daudé 
> Subject: [PATCH-for-9.1 05/21] cpus: Open code OBJECT_DECLARE_TYPE() in
>  OBJECT_DECLARE_CPU_TYPE()
> X-Mailer: git-send-email 2.41.0
> 
> Since the OBJECT_DECLARE_CPU_TYPE() macro uses the abstract ArchCPU
> type, when declaring multiple CPUs of the same ArchCPU type we get
> an error related to the indirect G_DEFINE_AUTOPTR_CLEANUP_FUNC()
> use within OBJECT_DECLARE_TYPE():
> 
>   target/mips/cpu-qom.h:31:1: error: redefinition of 
> 'glib_autoptr_clear_ArchCPU'
>   OBJECT_DECLARE_CPU_TYPE(MIPS64CPU, MIPSCPUClass, MIPS64_CPU)
>   ^
>   include/hw/core/cpu.h:82:5: note: expanded from macro 
> 'OBJECT_DECLARE_CPU_TYPE'
>   OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
>   ^
>   include/qom/object.h:237:5: note: expanded from macro 'OBJECT_DECLARE_TYPE'
>   G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \
>   ^
>   /usr/include/glib-2.0/glib/gmacros.h:1371:3: note: expanded from macro 
> 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
> _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func)
> ^
>   /usr/include/glib-2.0/glib/gmacros.h:1354:36: note: expanded from macro 
> '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS'
> static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) 
> (TypeName *_ptr) \
>  ^
>   /usr/include/glib-2.0/glib/gmacros.h:1338:49: note: expanded from macro 
> '_GLIB_AUTOPTR_CLEAR_FUNC_NAME'
>   #define _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) 
> glib_autoptr_clear_##TypeName
>   ^
>   :54:1: note: expanded from here
>   glib_autoptr_clear_ArchCPU
>   ^
>   target/mips/cpu-qom.h:30:1: note: previous definition is here
>   OBJECT_DECLARE_CPU_TYPE(MIPS32CPU, MIPSCPUClass, MIPS32_CPU)
>   ^
> 
> Avoid that problem by expanding the OBJECT_DECLARE_TYPE() macro
> within OBJECT_DECLARE_CPU_TYPE().
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Acked-by: Richard Henderson 
> ---
> TODO: check rth comment:
> What about adding an OBJECT_DECLARE_CPU_SUBTYPE that omits half the stuff 
> instead?
> We don't need another object typedef at all, for instance.
> ---
>  include/hw/core/cpu.h | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index ec14f74ce5..4c2e5095bf 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -78,7 +78,12 @@ DECLARE_CLASS_CHECKERS(CPUClass, CPU,
>   */
>  #define OBJECT_DECLARE_CPU_TYPE(CpuInstanceType, CpuClassType, 
> CPU_MODULE_OBJ_NAME) \
>  typedef struct ArchCPU CpuInstanceType; \
> -OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
> +typedef struct CpuClassType CpuClassType; \
> +\
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(CpuInstanceType, object_unref) \
> +\
> +DECLARE_OBJ_CHECKERS(CpuInstanceType, CpuClassType, \
> + CPU_MODULE_OBJ_NAME, TYPE_##CPU_MODULE_OBJ_NAME)
>

The OBJECT_DECLARE_TYPE is expaneded as the following:

#define OBJECT_DECLARE_TYPE(InstanceType, ClassType, MODULE_OBJ_NAME) \
typedef struct InstanceType InstanceType; \
typedef struct ClassType ClassType; \
\
G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \
\
DECLARE_OBJ_CHECKERS(InstanceType, ClassType, \
 MODULE_OBJ_NAME, TYPE_##MODULE_OBJ_NAME)

So the above code change deletes a typedef:

typedef struct ArchCPU ArchCPU;

Will this deletion break the direct uses of ArchCPU? e.g., in
accel/tcg/translator.c:

static void set_can_do_io(DisasContextBase *db, bool val)
{
if (db->saved_can_do_io != val) {
...
tcg_gen_st8_i32(tcg_constant_i32(val), tcg_env,
offsetof(ArchCPU, parent_obj.neg.can_do_io) -
offsetof(ArchCPU, env));
}
}

Thanks,
Zhao

Re: [PATCH 1/4] ui/vc: Do not inherit the size of active console

[Marc-André Lureau]

On Mon, Mar 18, 2024 at 11:59 AM Akihiko Odaki  wrote:
>
> A chardev-vc used to inherit the size of a graphic console when its
> size not explicitly specified, but it often did not make sense. If a
> chardev-vc is instantiated during the startup, the active graphic
> console has no content at the time, so it will have the size of graphic
> console placeholder, which contains no useful information. It's better
> to have the standard size of text console instead.
>
> Signed-off-by: Akihiko Odaki 

Reviewed-by: Marc-André Lureau 

> ---
>  ui/console-vc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ui/console-vc.c b/ui/console-vc.c
> index 9c13cc2981b0..f22c8e23c2ed 100644
> --- a/ui/console-vc.c
> +++ b/ui/console-vc.c
> @@ -990,8 +990,8 @@ static void vc_chr_open(Chardev *chr,
>  trace_console_txt_new(width, height);
>  if (width == 0 || height == 0) {
>  s = QEMU_TEXT_CONSOLE(object_new(TYPE_QEMU_TEXT_CONSOLE));
> -width = qemu_console_get_width(NULL, 80 * FONT_WIDTH);
> -height = qemu_console_get_height(NULL, 24 * FONT_HEIGHT);
> +width = 80 * FONT_WIDTH;
> +height = 24 * FONT_HEIGHT;
>  } else {
>  s = QEMU_TEXT_CONSOLE(object_new(TYPE_QEMU_FIXED_TEXT_CONSOLE));
>  }
>
> --
> 2.44.0
>
>


-- 
Marc-André Lureau

[PATCH 1/5] hw/loongarch: Refine acpi srat table for numa memory

[Bibo Mao]

One LoongArch virt machine platform, there is limitation for memory
map information. The minimum memory size is 256M and minimum memory
size for numa node0 is 256M also. With qemu numa qtest, it is possible
that memory size of numa node0 is 128M.

Limitations for minimum memory size for both total memory and numa
node0 is removed for acpi srat table creation.

Signed-off-by: Bibo Mao 
---
 hw/loongarch/acpi-build.c | 58 +++
 1 file changed, 34 insertions(+), 24 deletions(-)

diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index e5ab1080af..d0247d93ee 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -165,8 +165,9 @@ static void
 build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
 int i, arch_id, node_id;
-uint64_t mem_len, mem_base;
-int nb_numa_nodes = machine->numa_state->num_nodes;
+hwaddr len, base, gap;
+NodeInfo *numa_info;
+int nodes, nb_numa_nodes = machine->numa_state->num_nodes;
 LoongArchMachineState *lams = LOONGARCH_MACHINE(machine);
 MachineClass *mc = MACHINE_GET_CLASS(lams);
 const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(machine);
@@ -195,35 +196,44 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 build_append_int_noprefix(table_data, 0, 4); /* Reserved */
 }
 
-/* Node0 */
-build_srat_memory(table_data, VIRT_LOWMEM_BASE, VIRT_LOWMEM_SIZE,
-  0, MEM_AFFINITY_ENABLED);
-mem_base = VIRT_HIGHMEM_BASE;
-if (!nb_numa_nodes) {
-mem_len = machine->ram_size - VIRT_LOWMEM_SIZE;
-} else {
-mem_len = machine->numa_state->nodes[0].node_mem - VIRT_LOWMEM_SIZE;
+base = VIRT_LOWMEM_BASE;
+gap = VIRT_LOWMEM_SIZE;
+numa_info = machine->numa_state->nodes;
+nodes = nb_numa_nodes;
+if (!nodes) {
+nodes = 1;
 }
-if (mem_len)
-build_srat_memory(table_data, mem_base, mem_len, 0, 
MEM_AFFINITY_ENABLED);
-
-/* Node1 - Nodemax */
-if (nb_numa_nodes) {
-mem_base += mem_len;
-for (i = 1; i < nb_numa_nodes; ++i) {
-if (machine->numa_state->nodes[i].node_mem > 0) {
-build_srat_memory(table_data, mem_base,
-  machine->numa_state->nodes[i].node_mem, i,
-  MEM_AFFINITY_ENABLED);
-mem_base += machine->numa_state->nodes[i].node_mem;
-}
+
+for (i = 0; i < nodes; i++) {
+if (nb_numa_nodes) {
+len = numa_info[i].node_mem;
+} else {
+len = machine->ram_size;
+}
+
+/*
+ * memory for the node splited into two part
+ *   lowram:  [base, +gap)
+ *   highram: [VIRT_HIGHMEM_BASE, +(len - gap))
+ */
+if (len >= gap) {
+build_srat_memory(table_data, base, len, i, MEM_AFFINITY_ENABLED);
+len -= gap;
+base = VIRT_HIGHMEM_BASE;
+gap = machine->ram_size - VIRT_LOWMEM_SIZE;
+}
+
+if (len) {
+build_srat_memory(table_data, base, len, i, MEM_AFFINITY_ENABLED);
+base += len;
+gap  -= len;
 }
 }
 
 if (machine->device_memory) {
 build_srat_memory(table_data, machine->device_memory->base,
   memory_region_size(>device_memory->mr),
-  nb_numa_nodes - 1,
+  nodes - 1,
   MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
 }
 
-- 
2.39.3

Re: [PATCH v2 2/3] qom/object_interfaces: Make object_set_properties_from_qdict return bool

[Zhao Liu]

On Mon, Mar 18, 2024 at 11:32:09AM +0800, Zhenzhong Duan wrote:
> Date: Mon, 18 Mar 2024 11:32:09 +0800
> From: Zhenzhong Duan 
> Subject: [PATCH v2 2/3] qom/object_interfaces: Make
>  object_set_properties_from_qdict return bool
> X-Mailer: git-send-email 2.34.1
> 
> Make object_set_properties_from_qdict() return bool, so that
> user_creatable_add_type() could check its return value instead
> of local_err pointer.
> 
> Opportunistically, do the same change to check return value of
> object_property_try_add_child() instead of local_err pointer.
> 
> Suggested-by: Zhao Liu 
> Signed-off-by: Zhenzhong Duan 
> ---
>  qom/object_interfaces.c | 21 ++---
>  1 file changed, 10 insertions(+), 11 deletions(-)

Reviewed-by: Zhao Liu 

Re: [PATCH for 9.0 v15 03/10] target/riscv/vector_helper.c: fix 'vmvr_v' memcpy endianess

[Alistair Francis]

On Fri, Mar 15, 2024 at 3:58 AM Daniel Henrique Barboza
 wrote:
>
> vmvr_v isn't handling the case where the host might be big endian and
> the bytes to be copied aren't sequential.
>
> Suggested-by: Richard Henderson 
> Fixes: f714361ed7 ("target/riscv: rvv-1.0: implement vstart CSR")
> Signed-off-by: Daniel Henrique Barboza 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/vector_helper.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index ca79571ae2..34ac4aa808 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -5075,9 +5075,17 @@ void HELPER(vmvr_v)(void *vd, void *vs2, CPURISCVState 
> *env, uint32_t desc)
>  uint32_t startb = env->vstart * sewb;
>  uint32_t i = startb;
>
> +if (HOST_BIG_ENDIAN && i % 8 != 0) {
> +uint32_t j = ROUND_UP(i, 8);
> +memcpy((uint8_t *)vd + H1(j - 1),
> +   (uint8_t *)vs2 + H1(j - 1),
> +   j - i);
> +i = j;
> +}
> +
>  memcpy((uint8_t *)vd + H1(i),
> (uint8_t *)vs2 + H1(i),
> -   maxsz - startb);
> +   maxsz - i);
>
>  env->vstart = 0;
>  }
> --
> 2.44.0
>
>

Re: [PATCH for 9.0 v15 04/10] target/riscv: always clear vstart in whole vec move insns

[Alistair Francis]

On Fri, Mar 15, 2024 at 3:58 AM Daniel Henrique Barboza
 wrote:
>
> These insns have 2 paths: we'll either have vstart already cleared if
> vstart_eq_zero or we'll do a brcond to check if vstart >= maxsz to call
> the 'vmvr_v' helper. The helper will clear vstart if it executes until
> the end, or if vstart >= vl.
>
> For starters, the check itself is wrong: we're checking vstart >= maxsz,
> when in fact we should use vstart in bytes, or 'startb' like 'vmvr_v' is
> calling, to do the comparison. But even after fixing the comparison we'll
> still need to clear vstart in the end, which isn't happening too.
>
> We want to make the helpers responsible to manage vstart, including
> these corner cases, precisely to avoid these situations:
>
> - remove the wrong vstart >= maxsz cond from the translation;
> - add a 'startb >= maxsz' cond in 'vmvr_v', and clear vstart if that
>   happens.
>
> This way we're now sure that vstart is being cleared in the end of the
> execution, regardless of the path taken.
>
> Fixes: f714361ed7 ("target/riscv: rvv-1.0: implement vstart CSR")
> Signed-off-by: Daniel Henrique Barboza 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.c.inc | 3 ---
>  target/riscv/vector_helper.c| 5 +
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index 8c16a9f5b3..52c26a7834 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -3664,12 +3664,9 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME * 
> a)   \
>   vreg_ofs(s, a->rs2), maxsz, maxsz);\
>  mark_vs_dirty(s);   \
>  } else {\
> -TCGLabel *over = gen_new_label();   \
> -tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, maxsz, over);  \
>  tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
> tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); 
> \
>  mark_vs_dirty(s);   \
> -gen_set_label(over);\
>  }   \
>  return true;\
>  }   \
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 34ac4aa808..bcc553c0e2 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -5075,6 +5075,11 @@ void HELPER(vmvr_v)(void *vd, void *vs2, CPURISCVState 
> *env, uint32_t desc)
>  uint32_t startb = env->vstart * sewb;
>  uint32_t i = startb;
>
> +if (startb >= maxsz) {
> +env->vstart = 0;
> +return;
> +}
> +
>  if (HOST_BIG_ENDIAN && i % 8 != 0) {
>  uint32_t j = ROUND_UP(i, 8);
>  memcpy((uint8_t *)vd + H1(j - 1),
> --
> 2.44.0
>
>

Re: [PATCH for-9.0 v3] vdpa-dev: Fix initialisation order to restore VDUSE compatibility

[Michael S. Tsirkin]

On Fri, Mar 15, 2024 at 04:59:49PM +0100, Kevin Wolf wrote:
> VDUSE requires that virtqueues are first enabled before the DRIVER_OK
> status flag is set; with the current API of the kernel module, it is
> impossible to enable the opposite order in our block export code because
> userspace is not notified when a virtqueue is enabled.
> 
> This requirement also mathces the normal initialisation order as done by
> the generic vhost code in QEMU. However, commit 6c482547 accidentally
> changed the order for vdpa-dev and broke access to VDUSE devices with
> this.
> 
> This changes vdpa-dev to use the normal order again and use the standard
> vhost callback .vhost_set_vring_enable for this. VDUSE devices can be
> used with vdpa-dev again after this fix.
> 
> vhost_net intentionally avoided enabling the vrings for vdpa and does
> this manually later while it does enable them for other vhost backends.
> Reflect this in the vhost_net code and return early for vdpa, so that
> the behaviour doesn't change for this device.
> 
> Cc: qemu-sta...@nongnu.org
> Fixes: 6c4825476a4351530bcac17abab72295b75ffe98

Wrong format for the fixes tag.

Fixes: 6c4825476a ("vdpa: move vhost_vdpa_set_vring_ready to the caller")

is the right one.


> Signed-off-by: Kevin Wolf 
> ---
> v2:
> - Actually make use of the @enable parameter
> - Change vhost_net to preserve the current behaviour
> 
> v3:
> - Updated trace point [Stefano]
> - Fixed typo in comment [Stefano]
> 
>  hw/net/vhost_net.c | 10 ++
>  hw/virtio/vdpa-dev.c   |  5 +
>  hw/virtio/vhost-vdpa.c | 29 ++---
>  hw/virtio/vhost.c  |  8 +++-
>  hw/virtio/trace-events |  2 +-
>  5 files changed, 45 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index e8e1661646..fd1a93701a 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -541,6 +541,16 @@ int vhost_set_vring_enable(NetClientState *nc, int 
> enable)
>  VHostNetState *net = get_vhost_net(nc);
>  const VhostOps *vhost_ops = net->dev.vhost_ops;
>  
> +/*
> + * vhost-vdpa network devices need to enable dataplane virtqueues after
> + * DRIVER_OK, so they can recover device state before starting dataplane.
> + * Because of that, we don't enable virtqueues here and leave it to
> + * net/vhost-vdpa.c.
> + */
> +if (nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> +return 0;
> +}
> +
>  nc->vring_enable = enable;
>  
>  if (vhost_ops && vhost_ops->vhost_set_vring_enable) {
> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> index eb9ecea83b..13e87f06f6 100644
> --- a/hw/virtio/vdpa-dev.c
> +++ b/hw/virtio/vdpa-dev.c
> @@ -253,14 +253,11 @@ static int vhost_vdpa_device_start(VirtIODevice *vdev, 
> Error **errp)
>  
>  s->dev.acked_features = vdev->guest_features;
>  
> -ret = vhost_dev_start(>dev, vdev, false);
> +ret = vhost_dev_start(>dev, vdev, true);
>  if (ret < 0) {
>  error_setg_errno(errp, -ret, "Error starting vhost");
>  goto err_guest_notifiers;
>  }
> -for (i = 0; i < s->dev.nvqs; ++i) {
> -vhost_vdpa_set_vring_ready(>vdpa, i);
> -}
>  s->started = true;
>  
>  /*
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index ddae494ca8..31453466af 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -886,19 +886,41 @@ static int vhost_vdpa_get_vq_index(struct vhost_dev 
> *dev, int idx)
>  return idx;
>  }
>  
> -int vhost_vdpa_set_vring_ready(struct vhost_vdpa *v, unsigned idx)
> +static int vhost_vdpa_set_vring_enable_one(struct vhost_vdpa *v, unsigned 
> idx,
> +   int enable)
>  {
>  struct vhost_dev *dev = v->dev;
>  struct vhost_vring_state state = {
>  .index = idx,
> -.num = 1,
> +.num = enable,
>  };
>  int r = vhost_vdpa_call(dev, VHOST_VDPA_SET_VRING_ENABLE, );
>  
> -trace_vhost_vdpa_set_vring_ready(dev, idx, r);
> +trace_vhost_vdpa_set_vring_enable_one(dev, idx, enable, r);
>  return r;
>  }
>  
> +static int vhost_vdpa_set_vring_enable(struct vhost_dev *dev, int enable)
> +{
> +struct vhost_vdpa *v = dev->opaque;
> +unsigned int i;
> +int ret;
> +
> +for (i = 0; i < dev->nvqs; ++i) {
> +ret = vhost_vdpa_set_vring_enable_one(v, i, enable);
> +if (ret < 0) {
> +return ret;
> +}
> +}
> +
> +return 0;
> +}
> +
> +int vhost_vdpa_set_vring_ready(struct vhost_vdpa *v, unsigned idx)
> +{
> +return vhost_vdpa_set_vring_enable_one(v, idx, 1);
> +}
> +
>  static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
> int fd)
>  {
> @@ -1514,6 +1536,7 @@ const VhostOps vdpa_ops = {
>  .vhost_set_features = vhost_vdpa_set_features,
>  .vhost_reset_device = vhost_vdpa_reset_device,
>  .vhost_get_vq_index = vhost_vdpa_get_vq_index,
> +

Re: [PATCH for-9.0 v3] vdpa-dev: Fix initialisation order to restore VDUSE compatibility

[Michael S. Tsirkin]

On Mon, Mar 18, 2024 at 12:31:26PM +0800, Jason Wang wrote:
> On Fri, Mar 15, 2024 at 11:59 PM Kevin Wolf  wrote:
> >
> > VDUSE requires that virtqueues are first enabled before the DRIVER_OK
> > status flag is set; with the current API of the kernel module, it is
> > impossible to enable the opposite order in our block export code because
> > userspace is not notified when a virtqueue is enabled.
> >
> > This requirement also mathces the normal initialisation order as done by
> > the generic vhost code in QEMU. However, commit 6c482547 accidentally
> > changed the order for vdpa-dev and broke access to VDUSE devices with
> > this.
> >
> > This changes vdpa-dev to use the normal order again and use the standard
> > vhost callback .vhost_set_vring_enable for this. VDUSE devices can be
> > used with vdpa-dev again after this fix.
> >
> > vhost_net intentionally avoided enabling the vrings for vdpa and does
> > this manually later while it does enable them for other vhost backends.
> > Reflect this in the vhost_net code and return early for vdpa, so that
> > the behaviour doesn't change for this device.
> >
> > Cc: qemu-sta...@nongnu.org
> > Fixes: 6c4825476a4351530bcac17abab72295b75ffe98
> > Signed-off-by: Kevin Wolf 
> > ---
> > v2:
> > - Actually make use of the @enable parameter
> > - Change vhost_net to preserve the current behaviour
> >
> > v3:
> > - Updated trace point [Stefano]
> > - Fixed typo in comment [Stefano]
> >
> >  hw/net/vhost_net.c | 10 ++
> >  hw/virtio/vdpa-dev.c   |  5 +
> >  hw/virtio/vhost-vdpa.c | 29 ++---
> >  hw/virtio/vhost.c  |  8 +++-
> >  hw/virtio/trace-events |  2 +-
> >  5 files changed, 45 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index e8e1661646..fd1a93701a 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -541,6 +541,16 @@ int vhost_set_vring_enable(NetClientState *nc, int 
> > enable)
> >  VHostNetState *net = get_vhost_net(nc);
> >  const VhostOps *vhost_ops = net->dev.vhost_ops;
> >
> > +/*
> > + * vhost-vdpa network devices need to enable dataplane virtqueues after
> > + * DRIVER_OK, so they can recover device state before starting 
> > dataplane.
> > + * Because of that, we don't enable virtqueues here and leave it to
> > + * net/vhost-vdpa.c.
> > + */
> > +if (nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > +return 0;
> > +}
> 
> I think we need some inputs from Eugenio, this is only needed for
> shadow virtqueue during live migration but not other cases.
> 
> Thanks


Yes I think we had a backend flag for this, right? Eugenio can you
comment please?

Re: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit' parameter

[Philippe Mathieu-Daudé]

On 18/3/24 09:58, Markus Armbruster wrote:

Philippe Mathieu-Daudé  writes:


On 12/3/24 16:26, Zhao Liu wrote:

On Tue, Mar 12, 2024 at 03:13:41PM +0100, Markus Armbruster wrote:

Date: Tue, 12 Mar 2024 15:13:41 +0100
From: Markus Armbruster 
Subject: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit'
   parameter

From: Philippe Mathieu-Daudé 

QERR_INVALID_PARAMETER_VALUE is defined as:

#define QERR_INVALID_PARAMETER_VALUE \
"Parameter '%s' expects %s"

The current error is formatted as:

"Parameter 'vcpu_dirty_limit' expects is invalid, it must greater then 1 
MB/s"

Replace by:

"Parameter 'vcpu_dirty_limit' is invalid, it must greater than 1 MB/s"


Is there a grammar error here? Maybe
s/it must greater/it must be greater/


Oops indeed!


What about dropping "is invalid, "?  I.e. go with

 "Parameter 'vcpu_dirty_limit' must be greater than 1 MB/s"


Yes.




Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Juan Quintela 
Signed-off-by: Markus Armbruster 
---
   migration/options.c | 5 ++---
   1 file changed, 2 insertions(+), 3 deletions(-)

Re: [PATCH v1 2/2] system/cpus: Fix resume_all_vcpus() under vCPU hotplug condition

[David Hildenbrand]

On 17.03.24 09:37, Keqian Zhu via wrote:

For vCPU being hotplugged, qemu_init_vcpu() is called. In this
function, we set vcpu state as stopped, and then wait vcpu thread
to be created.

As the vcpu state is stopped, it will inform us it has been created
and then wait on halt_cond. After we has realized vcpu object, we
will resume the vcpu thread.

However, during we wait vcpu thread to be created, the bql is
unlocked, and other thread is allowed to call resume_all_vcpus(),
which will resume the un-realized vcpu.

This fixes the issue by filter out un-realized vcpu during
resume_all_vcpus().


Similar question: is there a reproducer?

How could we currently hotplug a VCPU, and while it is being created, 
see pause_all_vcpus()/resume_all_vcpus() getting claled.


If I am not getting this wrong, there seems to be some other mechanism 
missing that makes sure that this cannot happen. Dropping the BQL 
half-way through creating a VCPU might be the problem.


--
Cheers,

David / dhildenb

Re: [PATCH-for-9.1 3/3] ui/console: Add 'rotate_arcdegree' field to allow per-console rotation

[Akihiko Odaki]

On 2024/03/18 19:05, Philippe Mathieu-Daudé wrote:

Add the 'rotate_arcdegree' field to QemuConsole and remove
the use of the 'graphic_rotate' global.


I think QemuGraphicConsole is a better place to put the field.

Regards,
Akihiko Odaki



Signed-off-by: Philippe Mathieu-Daudé 
---
  ui/console-priv.h | 1 +
  ui/console.c  | 7 +++
  2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui/console-priv.h b/ui/console-priv.h
index 88569ed2cc..6e54b476d9 100644
--- a/ui/console-priv.h
+++ b/ui/console-priv.h
@@ -31,6 +31,7 @@ struct QemuConsole {
  const GraphicHwOps *hw_ops;
  void *hw;
  CoQueue dump_queue;
+unsigned rotate_arcdegree;
  
  QTAILQ_ENTRY(QemuConsole) next;

  };
diff --git a/ui/console.c b/ui/console.c
index 84aee76846..a36674bacf 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -37,7 +37,6 @@
  #include "trace.h"
  #include "exec/memory.h"
  #include "qom/object.h"
-#include "sysemu/sysemu.h"
  
  #include "console-priv.h"
  
@@ -210,17 +209,17 @@ void qemu_console_set_window_id(QemuConsole *con, int window_id)
  
  void qemu_console_set_rotate(QemuConsole *con, unsigned arcdegree)

  {
-graphic_rotate = arcdegree;
+con->rotate_arcdegree = arcdegree;
  }
  
  bool qemu_console_is_rotated(QemuConsole *con)

  {
-return graphic_rotate != 0;
+return con->rotate_arcdegree != 0;
  }
  
  unsigned qemu_console_get_rotate_arcdegree(QemuConsole *con)

  {
-return graphic_rotate;
+return con->rotate_arcdegree;
  }
  
  void graphic_hw_invalidate(QemuConsole *con)

Re: [PATCH v4 14/25] memory: Add Error** argument to the global_dirty_log routines

[Cédric Le Goater]

--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2836,18 +2836,31 @@ static void 
migration_bitmap_clear_discarded_pages(RAMState *rs)
  
  static void ram_init_bitmaps(RAMState *rs)

  {
+Error *local_err = NULL;
+bool ret = true;
+
  qemu_mutex_lock_ramlist();
  
  WITH_RCU_READ_LOCK_GUARD() {

  ram_list_init_bitmaps();
  /* We don't use dirty log with background snapshots */
  if (!migrate_background_snapshot()) {
-memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
+ret = memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION,
+_err);
+if (!ret) {
+error_report_err(local_err);
+goto out_unlock;


Here we may need to free the bitmaps created in ram_list_init_bitmaps().

We can have a helper ram_bitmaps_destroy() for that.

One thing be careful is the new file_bmap can be created but missing in the
ram_save_cleanup(), it's because it's freed earlier.  IMHO if we will have
a new ram_bitmaps_destroy() we can unconditionally free file_bmap there
too, as if it's freed early g_free() is noop.


OK. Let's do that in a new prereq patch. I will change ram_state_init()
and xbzrle_init() to take an Error ** argument while at it.


Thanks,

C.

Re: [PATCH v3 for 9.1 0/6] virtio, vhost: Add VIRTIO_F_NOTIFICATION_DATA support

[Jonah Palmer]

On 3/16/24 11:45 AM, Jiri Pirko wrote:

Fri, Mar 15, 2024 at 05:55:51PM CET, jonah.pal...@oracle.com wrote:

The goal of these patches are to add support to a variety of virtio and
vhost devices for the VIRTIO_F_NOTIFICATION_DATA transport feature. This
feature indicates that a driver will pass extra data (instead of just a
virtqueue's index) when notifying the corresponding device.

The data passed in by the driver when this feature is enabled varies in
format depending on if the device is using a split or packed virtqueue
layout:

Split VQ
  - Upper 16 bits: shadow_avail_idx
  - Lower 16 bits: virtqueue index

Packed VQ
  - Upper 16 bits: 1-bit wrap counter & 15-bit shadow_avail_idx
  - Lower 16 bits: virtqueue index

Also, due to the limitations of ioeventfd not being able to carry the
extra provided by the driver, having both VIRTIO_F_NOTIFICATION_DATA
feature and ioeventfd enabled is a functional mismatch. The user must
explicitly disable ioeventfd for the device in the Qemu arguments when
using this feature, else the device will fail to complete realization.

For example, a device must explicitly enable notification_data as well
as disable ioeventfd:

-device virtio-scsi-pci,...,ioeventfd=off,notification_data=on

A significant aspect of this effort has been to maintain compatibility
across different backends. As such, the feature is offered by backend
devices only when supported, with fallback mechanisms where backend
support is absent.

v3: Validate VQ idx via. virtio_queue_get_num() (pci, mmio, ccw)
Rename virtio_queue_set_shadow_avail_data
Only pass in upper 16 bits of 32-bit extra data (was redundant)
Make notification compatibility check function static
Drop tags on patches 1/6, 3/6, and 4/6

v2: Don't disable ioeventfd by default, user must disable it
Drop tags on patch 2/6

Jonah Palmer (6):
  virtio/virtio-pci: Handle extra notification data
  virtio: Prevent creation of device using notification-data with ioeventfd
  virtio-mmio: Handle extra notification data
  virtio-ccw: Handle extra notification data
  vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature bits
  virtio: Add VIRTIO_F_NOTIFICATION_DATA property definition


Jonah, do you have kernel patches to add this feature as well?

Thanks!


Hi Jiri! I think there are already kernel patches for 
VIRTIO_F_NOTIFICATION_DATA, unless you're referring to something more 
specific that wasn't included in these patches:


[1]: virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
https://lore.kernel.org/lkml/20230324195029.2410503-1-vik...@daynix.com/

[2]: virtio-vdpa: add VIRTIO_F_NOTIFICATION_DATA feature support
https://lore.kernel.org/lkml/20230413081855.36643-3-alvaro.ka...@solid-run.com/

Jonah

Re: [PATCH] blockdev: Fix blockdev-snapshot-sync error reporting for no medium

[Kevin Wolf]

Am 06.03.2024 um 15:28 hat Markus Armbruster geschrieben:
> When external_snapshot_abort() rejects a BlockDriverState without a
> medium, it creates an error like this:
> 
> error_setg(errp, "Device '%s' has no medium", device);
> 
> Trouble is @device can be null.  My system formats null as "(null)",
> but other systems might crash.  Reproducer:
> 
> 1. Create a block device without a medium
> 
> -> {"execute": "blockdev-add", "arguments": {"driver": "host_cdrom", 
> "node-name": "blk0", "filename": "/dev/sr0"}}
> <- {"return": {}}
> 
> 3. Attempt to snapshot it
> 
> -> {"execute":"blockdev-snapshot-sync", "arguments": { "node-name": 
> "blk0", "snapshot-file":"/tmp/foo.qcow2","format":"qcow2"}}
> <- {"error": {"class": "GenericError", "desc": "Device '(null)' has no 
> medium"}}
> 
> Broken when commit 0901f67ecdb made @device optional.
> 
> Use bdrv_get_device_or_node_name() instead.  Now it fails as it
> should:
> 
> <- {"error": {"class": "GenericError", "desc": "Device 'blk0' has no 
> medium"}}
> 
> Fixes: 0901f67ecdb7 (qmp: Allow to take external snapshots on bs graphs node.)
> Signed-off-by: Markus Armbruster 

Thanks, applied to the block branch.

Kevin

Re: [PATCH 2/4] ui/vnc: Do not use console_select()

[Marc-André Lureau]

Hi

On Mon, Mar 18, 2024 at 1:05 PM Akihiko Odaki  wrote:
>
> On 2024/03/18 17:21, Marc-André Lureau wrote:
> > Hi
> >
> > On Mon, Mar 18, 2024 at 11:58 AM Akihiko Odaki  
> > wrote:
> >>
> >> console_select() is shared by other displays and a console_select() call
> >> from one of them triggers console switching also in ui/curses,
> >> circumventing key state reinitialization that needs to be performed in
> >> preparation and resulting in stuck keys.
> >>
> >> Use its internal state to track the current active console to prevent
> >> such a surprise console switch.
> >>
> >> Signed-off-by: Akihiko Odaki 
> >> ---
> >>   include/ui/console.h   |  1 +
> >>   include/ui/kbd-state.h | 11 +++
> >>   ui/console.c   | 12 
> >>   ui/kbd-state.c |  6 ++
> >>   ui/vnc.c   | 14 +-
> >>   5 files changed, 39 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/ui/console.h b/include/ui/console.h
> >> index a4a49ffc640c..a703f7466499 100644
> >> --- a/include/ui/console.h
> >> +++ b/include/ui/console.h
> >> @@ -413,6 +413,7 @@ void qemu_console_early_init(void);
> >>
> >>   void qemu_console_set_display_gl_ctx(QemuConsole *con, DisplayGLCtx 
> >> *ctx);
> >>
> >> +QemuConsole *qemu_console_lookup_first_graphic_console(void);
> >>   QemuConsole *qemu_console_lookup_by_index(unsigned int index);
> >>   QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t 
> >> head);
> >>   QemuConsole *qemu_console_lookup_by_device_name(const char *device_id,
> >> diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
> >> index fb79776128cf..1f37b932eb62 100644
> >> --- a/include/ui/kbd-state.h
> >> +++ b/include/ui/kbd-state.h
> >> @@ -99,4 +99,15 @@ bool qkbd_state_modifier_get(QKbdState *kbd, 
> >> QKbdModifier mod);
> >>*/
> >>   void qkbd_state_lift_all_keys(QKbdState *kbd);
> >>
> >> +/**
> >> + * qkbd_state_switch_console: Switch console.
> >> + *
> >> + * This sends key up events to the previous console for all keys which 
> >> are in
> >> + * down state to prevent keys being stuck, and remembers the new console.
> >> + *
> >> + * @kbd: state tracker state.
> >> + * @con: new QemuConsole for this state tracker.
> >> + */
> >> +void qkbd_state_switch_console(QKbdState *kbd, QemuConsole *con);
> >> +
> >>   #endif /* QEMU_UI_KBD_STATE_H */
> >> diff --git a/ui/console.c b/ui/console.c
> >> index 832055675c50..6bf02a23414c 100644
> >> --- a/ui/console.c
> >> +++ b/ui/console.c
> >> @@ -1325,6 +1325,18 @@ void graphic_console_close(QemuConsole *con)
> >>   dpy_gfx_replace_surface(con, surface);
> >>   }
> >>
> >> +QemuConsole *qemu_console_lookup_first_graphic_console(void)
> >> +{
> >> +QemuConsole *con;
> >> +
> >> +QTAILQ_FOREACH(con, , next) {
> >> +if (QEMU_IS_GRAPHIC_CONSOLE(con)) {
> >> +return con;
> >> +}
> >> +}
> >> +return NULL;
> >> +}
> >> +
> >>   QemuConsole *qemu_console_lookup_by_index(unsigned int index)
> >>   {
> >>   QemuConsole *con;
> >> diff --git a/ui/kbd-state.c b/ui/kbd-state.c
> >> index 62d42a7a22e1..52ed28b8a89b 100644
> >> --- a/ui/kbd-state.c
> >> +++ b/ui/kbd-state.c
> >> @@ -117,6 +117,12 @@ void qkbd_state_lift_all_keys(QKbdState *kbd)
> >>   }
> >>   }
> >>
> >> +void qkbd_state_switch_console(QKbdState *kbd, QemuConsole *con)
> >> +{
> >> +qkbd_state_lift_all_keys(kbd);
> >> +kbd->con = con;
> >> +}
> >> +
> >>   void qkbd_state_set_delay(QKbdState *kbd, int delay_ms)
> >>   {
> >>   kbd->key_delay_ms = delay_ms;
> >> diff --git a/ui/vnc.c b/ui/vnc.c
> >> index fc12b343e254..94564b196ba8 100644
> >> --- a/ui/vnc.c
> >> +++ b/ui/vnc.c
> >> @@ -1872,12 +1872,16 @@ static void do_key_event(VncState *vs, int down, 
> >> int keycode, int sym)
> >>   /* QEMU console switch */
> >>   switch (qcode) {
> >>   case Q_KEY_CODE_1 ... Q_KEY_CODE_9: /* '1' to '9' keys */
> >> -if (vs->vd->dcl.con == NULL && down &&
> >> +if (down &&
> >>   qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_CTRL) &&
> >>   qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_ALT)) {
> >> -/* Reset the modifiers sent to the current console */
> >> -qkbd_state_lift_all_keys(vs->vd->kbd);
> >> -console_select(qcode - Q_KEY_CODE_1);
> >> +QemuConsole *con = qemu_console_lookup_by_index(qcode - 
> >> Q_KEY_CODE_1);
> >> +if (con) {
> >> +unregister_displaychangelistener(>vd->dcl);
> >> +qkbd_state_switch_console(vs->vd->kbd, con);
> >> +vs->vd->dcl.con = con;
> >> +register_displaychangelistener(>vd->dcl);
> >> +}
> >>   return;
> >>   }
> >>   default:
> >> @@ -4206,7 +4210,7 @@ void vnc_display_open(const char *id, Error **errp)
> >>   goto fail;
> >>   }
> >>   } else {
> >> -con = NULL;
> >> +con = 

[RFC PATCH v8 22/23] target/arm: Add FEAT_NMI to max

[Jinjie Ruan via]

Enable FEAT_NMI on the 'max' CPU.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v3:
- Add Reviewed-by.
- Sorted to last.
---
 docs/system/arm/emulation.rst | 1 +
 target/arm/tcg/cpu64.c| 1 +
 2 files changed, 2 insertions(+)

diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst
index 2a7bbb82dc..a9ae7ede9f 100644
--- a/docs/system/arm/emulation.rst
+++ b/docs/system/arm/emulation.rst
@@ -64,6 +64,7 @@ the following architecture extensions:
 - FEAT_MTE (Memory Tagging Extension)
 - FEAT_MTE2 (Memory Tagging Extension)
 - FEAT_MTE3 (MTE Asymmetric Fault Handling)
+- FEAT_NMI (Non-maskable Interrupt)
 - FEAT_NV (Nested Virtualization)
 - FEAT_NV2 (Enhanced nested virtualization support)
 - FEAT_PACIMP (Pointer authentication - IMPLEMENTATION DEFINED algorithm)
diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
index 9f7a9f3d2c..62c4663512 100644
--- a/target/arm/tcg/cpu64.c
+++ b/target/arm/tcg/cpu64.c
@@ -1175,6 +1175,7 @@ void aarch64_max_tcg_initfn(Object *obj)
 t = FIELD_DP64(t, ID_AA64PFR1, RAS_FRAC, 0);  /* FEAT_RASv1p1 + 
FEAT_DoubleFault */
 t = FIELD_DP64(t, ID_AA64PFR1, SME, 1);   /* FEAT_SME */
 t = FIELD_DP64(t, ID_AA64PFR1, CSV2_FRAC, 0); /* FEAT_CSV2_2 */
+t = FIELD_DP64(t, ID_AA64PFR1, NMI, 1);   /* FEAT_NMI */
 cpu->isar.id_aa64pfr1 = t;
 
 t = cpu->isar.id_aa64mmfr0;
-- 
2.34.1

[RFC PATCH v8 03/23] target/arm: Add support for FEAT_NMI, Non-maskable Interrupt

[Jinjie Ruan via]

Add support for FEAT_NMI. NMI (FEAT_NMI) is an mandatory feature in
ARMv8.8-A and ARM v9.3-A.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v3:
- Add Reviewed-by.
- Adjust to before the MSR patches.
---
 target/arm/internals.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index dd3da211a3..516e0584bf 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1229,6 +1229,9 @@ static inline uint32_t aarch64_pstate_valid_mask(const 
ARMISARegisters *id)
 if (isar_feature_aa64_mte(id)) {
 valid |= PSTATE_TCO;
 }
+if (isar_feature_aa64_nmi(id)) {
+valid |= PSTATE_ALLINT;
+}
 
 return valid;
 }
-- 
2.34.1

[RFC PATCH v8 18/23] hw/intc/arm_gicv3: Handle icv_nmiar1_read() for icc_nmiar1_read()

[Jinjie Ruan via]

Implement icv_nmiar1_read() for icc_nmiar1_read(), so add definition for
ICH_LR_EL2.NMI and ICH_AP1R_EL2.NMI bit.

If FEAT_GICv3_NMI is supported, ich_ap_write() should consider ICH_AP1R_EL2.NMI
bit. In icv_activate_irq() and icv_eoir_write(), the ICH_AP1R_EL2.NMI bit
should be set or clear according to the Superpriority info.

By the way, add gicv3_icv_nmiar1_read trace event.

If the hpp irq is a NMI, the icv iar read should return 1022 and trap for
NMI again

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v8:
- Fix an unexpected interrupt bug when sending VNMI by running qemu VM.
v7:
- Add Reviewed-by.
v6:
- Implement icv_nmiar1_read().
---
 hw/intc/arm_gicv3_cpuif.c | 52 ++-
 hw/intc/gicv3_internal.h  |  3 +++
 hw/intc/trace-events  |  1 +
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index df82a413c6..d67d62359b 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -728,7 +728,7 @@ static uint64_t icv_hppir_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 return value;
 }
 
-static void icv_activate_irq(GICv3CPUState *cs, int idx, int grp)
+static void icv_activate_irq(GICv3CPUState *cs, int idx, int grp, bool nmi)
 {
 /* Activate the interrupt in the specified list register
  * by moving it from Pending to Active state, and update the
@@ -742,7 +742,12 @@ static void icv_activate_irq(GICv3CPUState *cs, int idx, 
int grp)
 
 cs->ich_lr_el2[idx] &= ~ICH_LR_EL2_STATE_PENDING_BIT;
 cs->ich_lr_el2[idx] |= ICH_LR_EL2_STATE_ACTIVE_BIT;
-cs->ich_apr[grp][regno] |= (1 << regbit);
+
+if (cs->gic->nmi_support) {
+cs->ich_apr[grp][regno] |= (1 << regbit) | (nmi ? ICH_AP1R_EL2_NMI : 
0);
+} else {
+cs->ich_apr[grp][regno] |= (1 << regbit);
+}
 }
 
 static void icv_activate_vlpi(GICv3CPUState *cs)
@@ -775,8 +780,10 @@ static uint64_t icv_iar_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 
 if (thisgrp == grp && icv_hppi_can_preempt(cs, lr)) {
 intid = ich_lr_vintid(lr);
-if (!gicv3_intid_is_special(intid)) {
-icv_activate_irq(cs, idx, grp);
+if (!gicv3_intid_is_special(intid) && !(lr & ICH_LR_EL2_NMI)) {
+icv_activate_irq(cs, idx, grp, false);
+} else if (lr & ICH_LR_EL2_NMI) {
+intid = INTID_NMI;
 } else {
 /* Interrupt goes from Pending to Invalid */
 cs->ich_lr_el2[idx] &= ~ICH_LR_EL2_STATE_PENDING_BIT;
@@ -797,8 +804,32 @@ static uint64_t icv_iar_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 
 static uint64_t icv_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-/* todo */
+GICv3CPUState *cs = icc_cs_from_env(env);
+int idx = hppvi_index(cs);
 uint64_t intid = INTID_SPURIOUS;
+
+if (idx >= 0 && idx != HPPVI_INDEX_VLPI) {
+uint64_t lr = cs->ich_lr_el2[idx];
+int thisgrp = (lr & ICH_LR_EL2_GROUP) ? GICV3_G1NS : GICV3_G0;
+
+if ((thisgrp == GICV3_G1NS) && (lr & ICH_LR_EL2_NMI)) {
+intid = ich_lr_vintid(lr);
+if (!gicv3_intid_is_special(intid)) {
+icv_activate_irq(cs, idx, GICV3_G1NS, true);
+} else {
+/* Interrupt goes from Pending to Invalid */
+cs->ich_lr_el2[idx] &= ~ICH_LR_EL2_STATE_PENDING_BIT;
+/* We will now return the (bogus) ID from the list register,
+ * as per the pseudocode.
+ */
+}
+}
+}
+
+trace_gicv3_icv_nmiar1_read(gicv3_redist_affid(cs), intid);
+
+gicv3_cpuif_virt_update(cs);
+
 return intid;
 }
 
@@ -1403,6 +1434,11 @@ static int icv_drop_prio(GICv3CPUState *cs)
 return (apr0count + i * 32) << (icv_min_vbpr(cs) + 1);
 } else {
 *papr1 &= *papr1 - 1;
+
+if (cs->gic->nmi_support && (*papr1 & ICH_AP1R_EL2_NMI)) {
+*papr1 &= ~ICH_AP1R_EL2_NMI;
+}
+
 return (apr1count + i * 32) << (icv_min_vbpr(cs) + 1);
 }
 }
@@ -2552,7 +2588,11 @@ static void ich_ap_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 
 trace_gicv3_ich_ap_write(ri->crm & 1, regno, gicv3_redist_affid(cs), 
value);
 
-cs->ich_apr[grp][regno] = value & 0xU;
+if (cs->gic->nmi_support) {
+cs->ich_apr[grp][regno] = value & (0xU | ICH_AP1R_EL2_NMI);
+} else {
+cs->ich_apr[grp][regno] = value & 0xU;
+}
 gicv3_cpuif_virt_irq_fiq_update(cs);
 }
 
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 93e56b3726..5e2b32861d 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -242,6 +242,7 @@ FIELD(GICR_VPENDBASER, VALID, 63, 1)
 #define ICH_LR_EL2_PRIORITY_SHIFT 48
 #define ICH_LR_EL2_PRIORITY_LENGTH 8
 #define ICH_LR_EL2_PRIORITY_MASK (0xffULL << ICH_LR_EL2_PRIORITY_SHIFT)
+#define 

[RFC PATCH v8 15/23] hw/intc/arm_gicv3: Implement GICD_INMIR

[Jinjie Ruan via]

Add GICD_INMIR, GICD_INMIRnE register and support access GICD_INMIR0.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v4:
- Make the GICD_INMIR implementation more clearer.
- Udpate the commit message.
v3:
- Add Reviewed-by.
---
 hw/intc/arm_gicv3_dist.c | 34 ++
 hw/intc/gicv3_internal.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index 35e850685c..9739404e35 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -89,6 +89,29 @@ static int gicd_ns_access(GICv3State *s, int irq)
 return extract32(s->gicd_nsacr[irq / 16], (irq % 16) * 2, 2);
 }
 
+static void gicd_write_bitmap_reg(GICv3State *s, MemTxAttrs attrs,
+  uint32_t *bmp, maskfn *maskfn,
+  int offset, uint32_t val)
+{
+/*
+ * Helper routine to implement writing to a "set" register
+ * (GICD_INMIR, etc).
+ * Semantics implemented here:
+ * RAZ/WI for SGIs, PPIs, unimplemented IRQs
+ * Bits corresponding to Group 0 or Secure Group 1 interrupts RAZ/WI.
+ * offset should be the offset in bytes of the register from the start
+ * of its group.
+ */
+int irq = offset * 8;
+
+if (irq < GIC_INTERNAL || irq >= s->num_irq) {
+return;
+}
+val &= mask_group_and_nsacr(s, attrs, maskfn, irq);
+*gic_bmp_ptr32(bmp, irq) = val;
+gicv3_update(s, irq, 32);
+}
+
 static void gicd_write_set_bitmap_reg(GICv3State *s, MemTxAttrs attrs,
   uint32_t *bmp,
   maskfn *maskfn,
@@ -543,6 +566,11 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
 /* RAZ/WI since affinity routing is always enabled */
 *data = 0;
 return true;
+case GICD_INMIR ... GICD_INMIR + 0x7f:
+*data = (!s->nmi_support) ? 0 :
+gicd_read_bitmap_reg(s, attrs, s->superprio, NULL,
+ offset - GICD_INMIR);
+return true;
 case GICD_IROUTER ... GICD_IROUTER + 0x1fdf:
 {
 uint64_t r;
@@ -752,6 +780,12 @@ static bool gicd_writel(GICv3State *s, hwaddr offset,
 case GICD_SPENDSGIR ... GICD_SPENDSGIR + 0xf:
 /* RAZ/WI since affinity routing is always enabled */
 return true;
+case GICD_INMIR ... GICD_INMIR + 0x7f:
+if (s->nmi_support) {
+gicd_write_bitmap_reg(s, attrs, s->superprio, NULL,
+  offset - GICD_INMIR, value);
+}
+return true;
 case GICD_IROUTER ... GICD_IROUTER + 0x1fdf:
 {
 uint64_t r;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index f35b7d2f03..a1fc34597e 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -52,6 +52,8 @@
 #define GICD_SGIR0x0F00
 #define GICD_CPENDSGIR   0x0F10
 #define GICD_SPENDSGIR   0x0F20
+#define GICD_INMIR   0x0F80
+#define GICD_INMIRnE 0x3B00
 #define GICD_IROUTER 0x6000
 #define GICD_IDREGS  0xFFD0
 
-- 
2.34.1

[RFC PATCH v8 23/23] hw/arm/virt: Add FEAT_GICv3_NMI feature support in virt GIC

[Jinjie Ruan via]

A PE that implements FEAT_NMI and FEAT_GICv3 also implements
FEAT_GICv3_NMI. A PE that does not implement FEAT_NMI, does not implement
FEAT_GICv3_NMI

So included support FEAT_GICv3_NMI feature as part of virt platform
GIC initialization if FEAT_NMI and FEAT_GICv3 supported.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v4:
- Add Reviewed-by.
v3:
- Adjust to be the last after add FEAT_NMI to max.
- Check whether support FEAT_NMI and FEAT_GICv3 for FEAT_GICv3_NMI.
---
 hw/arm/virt.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index dc7959e164..680a8a3eb6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -729,6 +729,19 @@ static void create_v2m(VirtMachineState *vms)
 vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
 }
 
+/*
+ * A PE that implements FEAT_NMI and FEAT_GICv3 also implements
+ * FEAT_GICv3_NMI. A PE that does not implement FEAT_NMI, does not implement
+ * FEAT_GICv3_NMI.
+ */
+static bool gicv3_nmi_present(VirtMachineState *vms)
+{
+ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0));
+
+return cpu_isar_feature(aa64_nmi, cpu) &&
+   (vms->gic_version != VIRT_GIC_VERSION_2);
+}
+
 static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
 {
 MachineState *ms = MACHINE(vms);
@@ -802,6 +815,11 @@ static void create_gic(VirtMachineState *vms, MemoryRegion 
*mem)
   vms->virt);
 }
 }
+
+if (gicv3_nmi_present(vms)) {
+qdev_prop_set_bit(vms->gic, "has-nmi", true);
+}
+
 gicbusdev = SYS_BUS_DEVICE(vms->gic);
 sysbus_realize_and_unref(gicbusdev, _fatal);
 sysbus_mmio_map(gicbusdev, 0, vms->memmap[VIRT_GIC_DIST].base);
-- 
2.34.1

[RFC PATCH v8 04/23] target/arm: Implement ALLINT MSR (immediate)

[Jinjie Ruan via]

Add ALLINT MSR (immediate) to decodetree, in which the CRm is 0b000x. The
EL0 check is necessary to ALLINT, and the EL1 check is necessary when
imm == 1. So implement it inline for EL2/3, or EL1 with imm==0. Avoid the
unconditional write to pc and use raise_exception_ra to unwind.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v7:
- Add Reviewed-by.
v6:
- Fix DISAS_TOO_MANY to DISAS_UPDATE_EXIT and add the comment.
v5:
- Drop the & 1 in trans_MSR_i_ALLINT().
- Simplify and merge msr_i_allint() and allint_check().
- Rename msr_i_allint() to msr_set_allint_el1().
v4:
- Fix the ALLINT MSR (immediate) decodetree implementation.
- Remove arm_is_el2_enabled() check in allint_check().
- Update env->allint to env->pstate.
- Only call allint_check() when imm == 1.
- Simplify the allint_check() to not pass "op" and extract.
- Implement it inline for EL2/3, or EL1 with imm==0.
- Pass (a->imm & 1) * PSTATE_ALLINT (i64) to simplfy the ALLINT set/clear.
v3:
- Remove EL0 check in allint_check().
- Add TALLINT check for EL1 in allint_check().
- Remove unnecessarily arm_rebuild_hflags() in msr_i_allint helper.
---
 target/arm/tcg/a64.decode  |  1 +
 target/arm/tcg/helper-a64.c| 12 
 target/arm/tcg/helper-a64.h|  1 +
 target/arm/tcg/translate-a64.c | 19 +++
 4 files changed, 33 insertions(+)

diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 8a20dce3c8..0e7656fd15 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -207,6 +207,7 @@ MSR_i_DIT   1101 0101  0 011 0100  010 1 
@msr_i
 MSR_i_TCO   1101 0101  0 011 0100  100 1 @msr_i
 MSR_i_DAIFSET   1101 0101  0 011 0100  110 1 @msr_i
 MSR_i_DAIFCLEAR 1101 0101  0 011 0100  111 1 @msr_i
+MSR_i_ALLINT1101 0101  0 001 0100 000 imm:1 000 1
 MSR_i_SVCR  1101 0101  0 011 0100 0 mask:2 imm:1 011 1
 
 # MRS, MSR (register), SYS, SYSL. These are all essentially the
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index ebaa7f00df..7818537890 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -66,6 +66,18 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t imm)
 update_spsel(env, imm);
 }
 
+void HELPER(msr_set_allint_el1)(CPUARMState *env)
+{
+/* ALLINT update to PSTATE. */
+if (arm_hcrx_el2_eff(env) & HCRX_TALLINT) {
+raise_exception_ra(env, EXCP_UDEF,
+   syn_aa64_sysregtrap(0, 1, 0, 4, 1, 0x1f, 0),
+   exception_target_el(env), GETPC());
+}
+
+env->pstate |= PSTATE_ALLINT;
+}
+
 static void daif_check(CPUARMState *env, uint32_t op,
uint32_t imm, uintptr_t ra)
 {
diff --git a/target/arm/tcg/helper-a64.h b/target/arm/tcg/helper-a64.h
index 575a5dab7d..0518165399 100644
--- a/target/arm/tcg/helper-a64.h
+++ b/target/arm/tcg/helper-a64.h
@@ -22,6 +22,7 @@ DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64)
 DEF_HELPER_2(msr_i_spsel, void, env, i32)
 DEF_HELPER_2(msr_i_daifset, void, env, i32)
 DEF_HELPER_2(msr_i_daifclear, void, env, i32)
+DEF_HELPER_1(msr_set_allint_el1, void, env)
 DEF_HELPER_3(vfp_cmph_a64, i64, f16, f16, ptr)
 DEF_HELPER_3(vfp_cmpeh_a64, i64, f16, f16, ptr)
 DEF_HELPER_3(vfp_cmps_a64, i64, f32, f32, ptr)
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 340265beb0..21758b290d 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -2036,6 +2036,25 @@ static bool trans_MSR_i_DAIFCLEAR(DisasContext *s, arg_i 
*a)
 return true;
 }
 
+static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
+{
+if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) {
+return false;
+}
+
+if (a->imm == 0) {
+clear_pstate_bits(PSTATE_ALLINT);
+} else if (s->current_el > 1) {
+set_pstate_bits(PSTATE_ALLINT);
+} else {
+gen_helper_msr_set_allint_el1(tcg_env);
+}
+
+/* Exit the cpu loop to re-evaluate pending IRQs. */
+s->base.is_jmp = DISAS_UPDATE_EXIT;
+return true;
+}
+
 static bool trans_MSR_i_SVCR(DisasContext *s, arg_MSR_i_SVCR *a)
 {
 if (!dc_isar_feature(aa64_sme, s) || a->mask == 0) {
-- 
2.34.1

[RFC PATCH v8 10/23] hw/arm/virt: Wire NMI and VNMI irq lines from GIC to CPU

[Jinjie Ruan via]

Wire the new NMI and VNMI interrupt line from the GIC to each CPU.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v4:
- Add Reviewed-by.
v3:
- Also add VNMI wire.
---
 hw/arm/virt.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e5cd935232..dc7959e164 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -821,7 +821,8 @@ static void create_gic(VirtMachineState *vms, MemoryRegion 
*mem)
 
 /* Wire the outputs from each CPU's generic timer and the GICv3
  * maintenance interrupt signal to the appropriate GIC PPI inputs,
- * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
+ * and the GIC's IRQ/FIQ/VIRQ/VFIQ/NMI/VNMI interrupt outputs to the
+ * CPU's inputs.
  */
 for (i = 0; i < smp_cpus; i++) {
 DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
@@ -865,6 +866,10 @@ static void create_gic(VirtMachineState *vms, MemoryRegion 
*mem)
qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
 sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus,
+   qdev_get_gpio_in(cpudev, ARM_CPU_NMI));
+sysbus_connect_irq(gicbusdev, i + 5 * smp_cpus,
+   qdev_get_gpio_in(cpudev, ARM_CPU_VNMI));
 }
 
 fdt_add_gic_node(vms);
-- 
2.34.1

[RFC PATCH v8 08/23] target/arm: Handle IS/FS in ISR_EL1 for NMI

[Jinjie Ruan via]

Add IS and FS bit in ISR_EL1 and handle the read. With CPU_INTERRUPT_NMI or
CPU_INTERRUPT_VNMI, both CPSR_I and ISR_IS must be set. With
CPU_INTERRUPT_VFIQ and HCRX_EL2.VFNMI set, both CPSR_F and ISR_FS must be set.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v7:
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Add Reviewed-by.
v6:
- Verify that HCR_EL2.VF is set before checking VFNMI.
v4；
- Also handle VNMI.
v3:
- CPU_INTERRUPT_NMI do not set FIQ, so remove it.
- With CPU_INTERRUPT_NMI, both CPSR_I and ISR_IS must be set.
---
 target/arm/cpu.h|  2 ++
 target/arm/helper.c | 14 ++
 2 files changed, 16 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 629221e1a9..fba0a364db 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1396,6 +1396,8 @@ void pmu_init(ARMCPU *cpu);
 #define CPSR_N (1U << 31)
 #define CPSR_NZCV (CPSR_N | CPSR_Z | CPSR_C | CPSR_V)
 #define CPSR_AIF (CPSR_A | CPSR_I | CPSR_F)
+#define ISR_FS (1U << 9)
+#define ISR_IS (1U << 10)
 
 #define CPSR_IT (CPSR_IT_0_1 | CPSR_IT_2_7)
 #define CACHED_CPSR_BITS (CPSR_T | CPSR_AIF | CPSR_GE | CPSR_IT | CPSR_Q \
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5c637f3413..4bc63bf7ca 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2021,15 +2021,29 @@ static uint64_t isr_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 if (cs->interrupt_request & CPU_INTERRUPT_VIRQ) {
 ret |= CPSR_I;
 }
+if (cs->interrupt_request & CPU_INTERRUPT_VNMI) {
+ret |= ISR_IS;
+ret |= CPSR_I;
+}
 } else {
 if (cs->interrupt_request & CPU_INTERRUPT_HARD) {
 ret |= CPSR_I;
 }
+
+if (cs->interrupt_request & CPU_INTERRUPT_NMI) {
+ret |= ISR_IS;
+ret |= CPSR_I;
+}
 }
 
 if (hcr_el2 & HCR_FMO) {
 if (cs->interrupt_request & CPU_INTERRUPT_VFIQ) {
 ret |= CPSR_F;
+
+if ((arm_hcr_el2_eff(env) & HCR_VF) &&
+(arm_hcrx_el2_eff(env) & HCRX_VFNMI)) {
+ret |= ISR_FS;
+}
 }
 } else {
 if (cs->interrupt_request & CPU_INTERRUPT_FIQ) {
-- 
2.34.1

[RFC PATCH v8 11/23] hw/intc/arm_gicv3: Add external IRQ lines for NMI

[Jinjie Ruan via]

Augment the GICv3's QOM device interface by adding one
new set of sysbus IRQ line, to signal NMI to each CPU.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v4:
- Add Reviewed-by.
v3:
- Add support for VNMI.
---
 hw/intc/arm_gicv3_common.c | 6 ++
 include/hw/intc/arm_gic_common.h   | 2 ++
 include/hw/intc/arm_gicv3_common.h | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index cb55c72681..c52f060026 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -299,6 +299,12 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, 
qemu_irq_handler handler,
 for (i = 0; i < s->num_cpu; i++) {
 sysbus_init_irq(sbd, >cpu[i].parent_vfiq);
 }
+for (i = 0; i < s->num_cpu; i++) {
+sysbus_init_irq(sbd, >cpu[i].parent_nmi);
+}
+for (i = 0; i < s->num_cpu; i++) {
+sysbus_init_irq(sbd, >cpu[i].parent_vnmi);
+}
 
 memory_region_init_io(>iomem_dist, OBJECT(s), ops, s,
   "gicv3_dist", 0x1);
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index 7080375008..97fea4102d 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -71,6 +71,8 @@ struct GICState {
 qemu_irq parent_fiq[GIC_NCPU];
 qemu_irq parent_virq[GIC_NCPU];
 qemu_irq parent_vfiq[GIC_NCPU];
+qemu_irq parent_nmi[GIC_NCPU];
+qemu_irq parent_vnmi[GIC_NCPU];
 qemu_irq maintenance_irq[GIC_NCPU];
 
 /* GICD_CTLR; for a GIC with the security extensions the NS banked version
diff --git a/include/hw/intc/arm_gicv3_common.h 
b/include/hw/intc/arm_gicv3_common.h
index 4e2fb518e7..7324c7d983 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -155,6 +155,8 @@ struct GICv3CPUState {
 qemu_irq parent_fiq;
 qemu_irq parent_virq;
 qemu_irq parent_vfiq;
+qemu_irq parent_nmi;
+qemu_irq parent_vnmi;
 
 /* Redistributor */
 uint32_t level;  /* Current IRQ level */
-- 
2.34.1

Re: [PATCH 05/22] plugins: Move function pointer in qemu_plugin_dyn_cb

[Alex Bennée]

Richard Henderson  writes:

> The out-of-line function pointer is mutually exclusive
> with inline expansion, so move it into the union.
> Wrap the pointer in a structure named 'regular' to match
> PLUGIN_CB_REGULAR.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 for 9.1 0/6] virtio, vhost: Add VIRTIO_F_NOTIFICATION_DATA support

[Jiri Pirko]

Mon, Mar 18, 2024 at 12:22:02PM CET, jonah.pal...@oracle.com wrote:
>
>
>On 3/16/24 11:45 AM, Jiri Pirko wrote:
>> Fri, Mar 15, 2024 at 05:55:51PM CET, jonah.pal...@oracle.com wrote:
>> > The goal of these patches are to add support to a variety of virtio and
>> > vhost devices for the VIRTIO_F_NOTIFICATION_DATA transport feature. This
>> > feature indicates that a driver will pass extra data (instead of just a
>> > virtqueue's index) when notifying the corresponding device.
>> > 
>> > The data passed in by the driver when this feature is enabled varies in
>> > format depending on if the device is using a split or packed virtqueue
>> > layout:
>> > 
>> > Split VQ
>> >   - Upper 16 bits: shadow_avail_idx
>> >   - Lower 16 bits: virtqueue index
>> > 
>> > Packed VQ
>> >   - Upper 16 bits: 1-bit wrap counter & 15-bit shadow_avail_idx
>> >   - Lower 16 bits: virtqueue index
>> > 
>> > Also, due to the limitations of ioeventfd not being able to carry the
>> > extra provided by the driver, having both VIRTIO_F_NOTIFICATION_DATA
>> > feature and ioeventfd enabled is a functional mismatch. The user must
>> > explicitly disable ioeventfd for the device in the Qemu arguments when
>> > using this feature, else the device will fail to complete realization.
>> > 
>> > For example, a device must explicitly enable notification_data as well
>> > as disable ioeventfd:
>> > 
>> > -device virtio-scsi-pci,...,ioeventfd=off,notification_data=on
>> > 
>> > A significant aspect of this effort has been to maintain compatibility
>> > across different backends. As such, the feature is offered by backend
>> > devices only when supported, with fallback mechanisms where backend
>> > support is absent.
>> > 
>> > v3: Validate VQ idx via. virtio_queue_get_num() (pci, mmio, ccw)
>> > Rename virtio_queue_set_shadow_avail_data
>> > Only pass in upper 16 bits of 32-bit extra data (was redundant)
>> > Make notification compatibility check function static
>> > Drop tags on patches 1/6, 3/6, and 4/6
>> > 
>> > v2: Don't disable ioeventfd by default, user must disable it
>> > Drop tags on patch 2/6
>> > 
>> > Jonah Palmer (6):
>> >   virtio/virtio-pci: Handle extra notification data
>> >   virtio: Prevent creation of device using notification-data with ioeventfd
>> >   virtio-mmio: Handle extra notification data
>> >   virtio-ccw: Handle extra notification data
>> >   vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature bits
>> >   virtio: Add VIRTIO_F_NOTIFICATION_DATA property definition
>> 
>> Jonah, do you have kernel patches to add this feature as well?
>> 
>> Thanks!
>
>Hi Jiri! I think there are already kernel patches for
>VIRTIO_F_NOTIFICATION_DATA, unless you're referring to something more
>specific that wasn't included in these patches:
>
>[1]: virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
>https://lore.kernel.org/lkml/20230324195029.2410503-1-vik...@daynix.com/
>
>[2]: virtio-vdpa: add VIRTIO_F_NOTIFICATION_DATA feature support
>https://lore.kernel.org/lkml/20230413081855.36643-3-alvaro.ka...@solid-run.com/

I missed this. Thx!

>
>Jonah

Re: [PATCH for 9.0 v15 05/10] target/riscv: always clear vstart for ldst_whole insns

[Alistair Francis]

On Fri, Mar 15, 2024 at 3:57 AM Daniel Henrique Barboza
 wrote:
>
> Commit 8ff8ac6329 added a conditional to guard the vext_ldst_whole()
> helper if vstart >= evl. But by skipping the helper we're also not
> setting vstart = 0 at the end of the insns, which is incorrect.
>
> We'll move the conditional to vext_ldst_whole(), following in line with
> the removal of all brconds vstart >= vl that the next patch will do. The
> idea is to make the helpers responsible for their own vstart management.
>
> Fix ldst_whole isns by:
>
> - remove the brcond that skips the helper if vstart is >= evl;
>
> - vext_ldst_whole() now does an early exit with the same check, where
>   evl = (vlenb * nf) >> log2_esz, but the early exit will also clear
>   vstart.
>
> The 'width' param is now unneeded in ldst_whole_trans() and is also
> removed. It was used for the evl calculation for the brcond and has no
> other use now.  The 'width' is reflected in vext_ldst_whole() via
> log2_esz, which is encoded by GEN_VEXT_LD_WHOLE() as
> "ctzl(sizeof(ETYPE))".
>
> Suggested-by: Max Chou 
> Fixes: 8ff8ac6329 ("target/riscv: rvv: Add missing early exit condition for 
> whole register load/store")
> Signed-off-by: Daniel Henrique Barboza 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.c.inc | 52 +++--
>  target/riscv/vector_helper.c|  5 +++
>  2 files changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index 52c26a7834..1366445e1f 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -1097,13 +1097,9 @@ GEN_VEXT_TRANS(vle64ff_v, MO_64, r2nfvm, ldff_op, 
> ld_us_check)
>  typedef void gen_helper_ldst_whole(TCGv_ptr, TCGv, TCGv_env, TCGv_i32);
>
>  static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf,
> - uint32_t width, gen_helper_ldst_whole *fn,
> + gen_helper_ldst_whole *fn,
>   DisasContext *s)
>  {
> -uint32_t evl = s->cfg_ptr->vlenb * nf / width;
> -TCGLabel *over = gen_new_label();
> -tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, evl, over);
> -
>  TCGv_ptr dest;
>  TCGv base;
>  TCGv_i32 desc;
> @@ -1120,8 +1116,6 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, 
> uint32_t nf,
>
>  fn(dest, base, tcg_env, desc);
>
> -gen_set_label(over);
> -
>  return true;
>  }
>
> @@ -1129,42 +1123,42 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t 
> rs1, uint32_t nf,
>   * load and store whole register instructions ignore vtype and vl setting.
>   * Thus, we don't need to check vill bit. (Section 7.9)
>   */
> -#define GEN_LDST_WHOLE_TRANS(NAME, ARG_NF, WIDTH)   \
> +#define GEN_LDST_WHOLE_TRANS(NAME, ARG_NF)\
>  static bool trans_##NAME(DisasContext *s, arg_##NAME * a) \
>  { \
>  if (require_rvv(s) && \
>  QEMU_IS_ALIGNED(a->rd, ARG_NF)) { \
> -return ldst_whole_trans(a->rd, a->rs1, ARG_NF, WIDTH, \
> +return ldst_whole_trans(a->rd, a->rs1, ARG_NF,\
>  gen_helper_##NAME, s);\
>  } \
>  return false; \
>  }
>
> -GEN_LDST_WHOLE_TRANS(vl1re8_v,  1, 1)
> -GEN_LDST_WHOLE_TRANS(vl1re16_v, 1, 2)
> -GEN_LDST_WHOLE_TRANS(vl1re32_v, 1, 4)
> -GEN_LDST_WHOLE_TRANS(vl1re64_v, 1, 8)
> -GEN_LDST_WHOLE_TRANS(vl2re8_v,  2, 1)
> -GEN_LDST_WHOLE_TRANS(vl2re16_v, 2, 2)
> -GEN_LDST_WHOLE_TRANS(vl2re32_v, 2, 4)
> -GEN_LDST_WHOLE_TRANS(vl2re64_v, 2, 8)
> -GEN_LDST_WHOLE_TRANS(vl4re8_v,  4, 1)
> -GEN_LDST_WHOLE_TRANS(vl4re16_v, 4, 2)
> -GEN_LDST_WHOLE_TRANS(vl4re32_v, 4, 4)
> -GEN_LDST_WHOLE_TRANS(vl4re64_v, 4, 8)
> -GEN_LDST_WHOLE_TRANS(vl8re8_v,  8, 1)
> -GEN_LDST_WHOLE_TRANS(vl8re16_v, 8, 2)
> -GEN_LDST_WHOLE_TRANS(vl8re32_v, 8, 4)
> -GEN_LDST_WHOLE_TRANS(vl8re64_v, 8, 8)
> +GEN_LDST_WHOLE_TRANS(vl1re8_v,  1)
> +GEN_LDST_WHOLE_TRANS(vl1re16_v, 1)
> +GEN_LDST_WHOLE_TRANS(vl1re32_v, 1)
> +GEN_LDST_WHOLE_TRANS(vl1re64_v, 1)
> +GEN_LDST_WHOLE_TRANS(vl2re8_v,  2)
> +GEN_LDST_WHOLE_TRANS(vl2re16_v, 2)
> +GEN_LDST_WHOLE_TRANS(vl2re32_v, 2)
> +GEN_LDST_WHOLE_TRANS(vl2re64_v, 2)
> +GEN_LDST_WHOLE_TRANS(vl4re8_v,  4)
> +GEN_LDST_WHOLE_TRANS(vl4re16_v, 4)
> +GEN_LDST_WHOLE_TRANS(vl4re32_v, 4)
> +GEN_LDST_WHOLE_TRANS(vl4re64_v, 4)
> +GEN_LDST_WHOLE_TRANS(vl8re8_v,  8)
> +GEN_LDST_WHOLE_TRANS(vl8re16_v, 8)
> +GEN_LDST_WHOLE_TRANS(vl8re32_v, 8)
> +GEN_LDST_WHOLE_TRANS(vl8re64_v, 8)
>
>  /*
>   * The vector whole register store instructions are encoded similar to
>   * 

Re: [PATCH v5 25/25] qapi: Dumb down QAPISchema.lookup_entity()

[Markus Armbruster]

Markus Armbruster  writes:

> QAPISchema.lookup_entity() takes an optional type argument, a subtype
> of QAPISchemaDefinition, and returns that type or None.  Callers can
> use this to save themselves an isinstance() test.
>
> The only remaining user of this convenience feature is .lookup_type().
> But we don't actually save anything anymore there: we still the

we still need the

> isinstance() to help mypy over the hump.
>
> Drop the .lookup_entity() argument, and adjust .lookup_type().
>
> Signed-off-by: Markus Armbruster 

[PATCH-for-9.1 v2 0/3] ui/display: Introduce API to change console orientation

[Philippe Mathieu-Daudé]

Since v1:
- Move rotate_arcdegree to QemuGraphicConsole (Akihiko)

Hi,

The idea behind this series is to reduce the use of the
'graphic_rotate' global. It is only used by the Spitz
machine, so we could convert the '-rotate' argument to
a sugar property on the PXA2XX_LCD_TYPE model, but since
the Spitz machine has been deprecated recently (commit
a2531bb855 "Deprecate various old Arm machine types") it
doesn't seem worthwhile. So just extract the API to change
console orientation.

Regards,

Phil.

Philippe Mathieu-Daudé (3):
  ui/console: Introduce API to change console orientation
  hw/display/pxa2xx_lcd: Set rotation angle using
qemu_console_set_rotate
  ui/console: Add 'rotate_arcdegree' field to allow per-console rotation

 include/ui/console.h|  3 +++
 hw/display/pxa2xx_lcd.c |  1 +
 ui/console.c| 23 +++
 ui/input.c  |  9 -
 4 files changed, 31 insertions(+), 5 deletions(-)

-- 
2.41.0

[PULL 14/15] tests/qemu-iotests: Restrict tests using "--blockdev file" to the file protocol

[Kevin Wolf]

From: Thomas Huth 

Tests that use "--blockdev" with the "file" driver cannot work with
other protocols, so we should mark them accordingly.

Signed-off-by: Thomas Huth 
Message-ID: <2024031508.153201-10-th...@redhat.com>
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/tests/qcow2-internal-snapshots | 2 +-
 tests/qemu-iotests/tests/qsd-jobs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/tests/qcow2-internal-snapshots 
b/tests/qemu-iotests/tests/qcow2-internal-snapshots
index 36523aba06..9f83aa8903 100755
--- a/tests/qemu-iotests/tests/qcow2-internal-snapshots
+++ b/tests/qemu-iotests/tests/qcow2-internal-snapshots
@@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # This tests qcow2-specific low-level functionality
 _supported_fmt qcow2
-_supported_proto generic
+_supported_proto file
 # Internal snapshots are (currently) impossible with refcount_bits=1,
 # and generally impossible with external data files
 _unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' data_file
diff --git a/tests/qemu-iotests/tests/qsd-jobs 
b/tests/qemu-iotests/tests/qsd-jobs
index 510bf0a9dc..9b843af631 100755
--- a/tests/qemu-iotests/tests/qsd-jobs
+++ b/tests/qemu-iotests/tests/qsd-jobs
@@ -40,7 +40,7 @@ cd ..
 . ./common.filter
 
 _supported_fmt qcow2
-_supported_proto generic
+_supported_proto file
 
 size=128M
 
-- 
2.44.0

[PULL 02/15] nbd/server: Fix race in draining the export

[Kevin Wolf]

When draining an NBD export, nbd_drained_begin() first sets
client->quiescing so that nbd_client_receive_next_request() won't start
any new request coroutines. Then nbd_drained_poll() tries to makes sure
that we wait for any existing request coroutines by checking that
client->nb_requests has become 0.

However, there is a small window between creating a new request
coroutine and increasing client->nb_requests. If a coroutine is in this
state, it won't be waited for and drain returns too early.

In the context of switching to a different AioContext, this means that
blk_aio_attached() will see client->recv_coroutine != NULL and fail its
assertion.

Fix this by increasing client->nb_requests immediately when starting the
coroutine. Doing this after the checks if we should create a new
coroutine is okay because client->lock is held.

Cc: qemu-sta...@nongnu.org
Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server")
Signed-off-by: Kevin Wolf 
Message-ID: <20240314165825.40261-2-kw...@redhat.com>
Signed-off-by: Kevin Wolf 
---
 nbd/server.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 941832f178..c3484cc1eb 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -3007,8 +3007,8 @@ static coroutine_fn int nbd_handle_request(NBDClient 
*client,
 /* Owns a reference to the NBDClient passed as opaque.  */
 static coroutine_fn void nbd_trip(void *opaque)
 {
-NBDClient *client = opaque;
-NBDRequestData *req = NULL;
+NBDRequestData *req = opaque;
+NBDClient *client = req->client;
 NBDRequest request = { 0 };/* GCC thinks it can be used uninitialized 
*/
 int ret;
 Error *local_err = NULL;
@@ -3037,8 +3037,6 @@ static coroutine_fn void nbd_trip(void *opaque)
 goto done;
 }
 
-req = nbd_request_get(client);
-
 /*
  * nbd_co_receive_request() returns -EAGAIN when nbd_drained_begin() has
  * set client->quiescing but by the time we get back nbd_drained_end() may
@@ -3112,9 +3110,7 @@ static coroutine_fn void nbd_trip(void *opaque)
 }
 
 done:
-if (req) {
-nbd_request_put(req);
-}
+nbd_request_put(req);
 
 qemu_mutex_unlock(>lock);
 
@@ -3143,10 +3139,13 @@ disconnect:
  */
 static void nbd_client_receive_next_request(NBDClient *client)
 {
+NBDRequestData *req;
+
 if (!client->recv_coroutine && client->nb_requests < MAX_NBD_REQUESTS &&
 !client->quiescing) {
 nbd_client_get(client);
-client->recv_coroutine = qemu_coroutine_create(nbd_trip, client);
+req = nbd_request_get(client);
+client->recv_coroutine = qemu_coroutine_create(nbd_trip, req);
 aio_co_schedule(client->exp->common.ctx, client->recv_coroutine);
 }
 }
-- 
2.44.0

[PULL 15/15] iotests: adapt to output change for recently introduced 'detached header' field

[Kevin Wolf]

From: Fiona Ebner 

Failure was noticed when running the tests for the qcow2 image format.

Fixes: 0bd779e27e ("crypto: Introduce 'detached-header' field in 
QCryptoBlockInfoLUKS")
Signed-off-by: Fiona Ebner 
Message-ID: <20240216101415.293769-1-f.eb...@proxmox.com>
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/198.out | 2 ++
 tests/qemu-iotests/206.out | 1 +
 2 files changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out
index 805494916f..62fb73fa3e 100644
--- a/tests/qemu-iotests/198.out
+++ b/tests/qemu-iotests/198.out
@@ -39,6 +39,7 @@ Format specific information:
 compression type: COMPRESSION_TYPE
 encrypt:
 ivgen alg: plain64
+detached header: false
 hash alg: sha256
 cipher alg: aes-256
 uuid: ----
@@ -84,6 +85,7 @@ Format specific information:
 compression type: COMPRESSION_TYPE
 encrypt:
 ivgen alg: plain64
+detached header: false
 hash alg: sha256
 cipher alg: aes-256
 uuid: ----
diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
index 7e95694777..979f00f9bf 100644
--- a/tests/qemu-iotests/206.out
+++ b/tests/qemu-iotests/206.out
@@ -114,6 +114,7 @@ Format specific information:
 refcount bits: 16
 encrypt:
 ivgen alg: plain64
+detached header: false
 hash alg: sha1
 cipher alg: aes-128
 uuid: ----
-- 
2.44.0

Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap

[Eric Auger]

Hi Michael,

On 3/13/24 12:17, Michael S. Tsirkin wrote:
> On Wed, Mar 13, 2024 at 07:54:11AM +, Duan, Zhenzhong wrote:
>>
>>> -Original Message-
>>> From: Michael S. Tsirkin 
>>> Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>>> sync host IOMMU cap/ecap
>>>
>>> On Wed, Mar 13, 2024 at 02:52:39AM +, Duan, Zhenzhong wrote:
 Hi Michael,

> -Original Message-
> From: Michael S. Tsirkin 
> Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
> sync host IOMMU cap/ecap
>
> On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
>> From: Yi Liu 
>>
>> Add a framework to check and synchronize host IOMMU cap/ecap with
>> vIOMMU cap/ecap.
>>
>> The sequence will be:
>>
>> vtd_cap_init() initializes iommu->cap/ecap.
>> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
>> iommu->cap_frozen set when machine create done, iommu->cap/ecap
> become readonly.
>> Implementation details for different backends will be in following
>>> patches.
>> Signed-off-by: Yi Liu 
>> Signed-off-by: Yi Sun 
>> Signed-off-by: Zhenzhong Duan 
>> ---
>>  include/hw/i386/intel_iommu.h |  1 +
>>  hw/i386/intel_iommu.c | 50
> ++-
>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/i386/intel_iommu.h
> b/include/hw/i386/intel_iommu.h
>> index bbc7b96add..c71a133820 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
>>
>>  uint64_t cap;   /* The value of capability reg */
>>  uint64_t ecap;  /* The value of extended capability 
>> reg */
>> +bool cap_frozen;/* cap/ecap become read-only after
>>> frozen */
>>  uint32_t context_cache_gen; /* Should be in [1,MAX] */
>>  GHashTable *iotlb;  /* IOTLB */
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index ffa1ad6429..a9f9dfd6a7 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -35,6 +35,8 @@
>>  #include "sysemu/kvm.h"
>>  #include "sysemu/dma.h"
>>  #include "sysemu/sysemu.h"
>> +#include "hw/vfio/vfio-common.h"
>> +#include "sysemu/iommufd.h"
>>  #include "hw/i386/apic_internal.h"
>>  #include "kvm/kvm_i386.h"
>>  #include "migration/vmstate.h"
>> @@ -3819,6 +3821,38 @@ VTDAddressSpace
> *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>>  return vtd_dev_as;
>>  }
>>
>> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
>> + IOMMULegacyDevice *ldev,
>> + Error **errp)
>> +{
>> +return 0;
>> +}
>> +
>> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>> +  IOMMUFDDevice *idev,
>> +  Error **errp)
>> +{
>> +return 0;
>> +}
>> +
>> +static int vtd_check_hdev(IntelIOMMUState *s,
>>> VTDHostIOMMUDevice
> *vtd_hdev,
>> +  Error **errp)
>> +{
>> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
>> +IOMMUFDDevice *idev;
>> +
>> +if (base_dev->type == HID_LEGACY) {
>> +IOMMULegacyDevice *ldev = container_of(base_dev,
>> +   IOMMULegacyDevice, base);
>> +
>> +return vtd_check_legacy_hdev(s, ldev, errp);
>> +}
>> +
>> +idev = container_of(base_dev, IOMMUFDDevice, base);
>> +
>> +return vtd_check_iommufd_hdev(s, idev, errp);
>> +}
>> +
>>  static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
> devfn,
>>  HostIOMMUDevice *base_dev, Error 
>> **errp)
>>  {
>> @@ -3829,6 +3863,7 @@ static int
>>> vtd_dev_set_iommu_device(PCIBus
> *bus, void *opaque, int devfn,
>>  .devfn = devfn,
>>  };
>>  struct vtd_as_key *new_key;
>> +int ret;
>>
>>  assert(base_dev);
>>
>> @@ -3848,6 +3883,13 @@ static int
>>> vtd_dev_set_iommu_device(PCIBus
> *bus, void *opaque, int devfn,
>>  vtd_hdev->iommu_state = s;
>>  vtd_hdev->dev = base_dev;
>>
>> +ret = vtd_check_hdev(s, vtd_hdev, errp);
>> +if (ret) {
>> +g_free(vtd_hdev);
>> +vtd_iommu_unlock(s);
>> +return ret;
>> +}
>> +
>>  new_key = g_malloc(sizeof(*new_key));
>>  new_key->bus = bus;
>>  new_key->devfn = devfn;
>
> Okay. So when VFIO device is created, it will call
>>> vtd_dev_set_iommu_device
> and that in turn will update caps.
>
>
>

[RFC PATCH v8 01/23] target/arm: Handle HCR_EL2 accesses for bits introduced with FEAT_NMI

[Jinjie Ruan via]

FEAT_NMI defines another three new bits in HCRX_EL2: TALLINT, HCRX_VINMI and
HCRX_VFNMI. When the feature is enabled, allow these bits to be written in
HCRX_EL2.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v4:
- Update the comment for FEAT_NMI in hcrx_write().
- Update the commit message, s/thress/three/g.
v3:
- Add Reviewed-by.
- Add HCRX_VINMI and HCRX_VFNMI support in HCRX_EL2.
- Upate the commit messsage.
---
 target/arm/cpu-features.h | 5 +
 target/arm/helper.c   | 5 +
 2 files changed, 10 insertions(+)

diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index e5758d9fbc..b300d0446d 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -681,6 +681,11 @@ static inline bool isar_feature_aa64_sme(const 
ARMISARegisters *id)
 return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, SME) != 0;
 }
 
+static inline bool isar_feature_aa64_nmi(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, NMI) != 0;
+}
+
 static inline bool isar_feature_aa64_tgran4_lpa2(const ARMISARegisters *id)
 {
 return FIELD_SEX64(id->id_aa64mmfr0, ID_AA64MMFR0, TGRAN4) >= 1;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 3f3a5b55d4..b19a0178ce 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6190,6 +6190,11 @@ static void hcrx_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 valid_mask |= HCRX_MSCEN | HCRX_MCE2;
 }
 
+/* FEAT_NMI adds TALLINT, VINMI and VFNMI */
+if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) {
+valid_mask |= HCRX_TALLINT | HCRX_VINMI | HCRX_VFNMI;
+}
+
 /* Clear RES0 bits.  */
 env->cp15.hcrx_el2 = value & valid_mask;
 }
-- 
2.34.1

[RFC PATCH v8 02/23] target/arm: Add PSTATE.ALLINT

[Jinjie Ruan via]

When PSTATE.ALLINT is set, an IRQ or FIQ interrupt that is targeted to
ELx, with or without superpriority is masked.

As Richard suggested, place ALLINT bit in PSTATE in env->pstate.

With the change to pstate_read/write, exception entry
and return are automatically handled.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v5:
- Remove the ALLINT comment, as it is covered by "all other bits".
- Add Reviewed-by.
v4:
- Keep PSTATE.ALLINT in env->pstate but not env->allint.
- Update the commit message.
v3:
- Remove ALLINT dump in aarch64_cpu_dump_state().
- Update the commit message.
---
 target/arm/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index bc0c84873f..de740d223f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1430,6 +1430,7 @@ void pmu_init(ARMCPU *cpu);
 #define PSTATE_D (1U << 9)
 #define PSTATE_BTYPE (3U << 10)
 #define PSTATE_SSBS (1U << 12)
+#define PSTATE_ALLINT (1U << 13)
 #define PSTATE_IL (1U << 20)
 #define PSTATE_SS (1U << 21)
 #define PSTATE_PAN (1U << 22)
-- 
2.34.1

Re: [PATCH 04/22] plugins: Zero new qemu_plugin_dyn_cb entries

[Alex Bennée]

Richard Henderson  writes:

> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 00/10] Reduce usage of QERR_ macros further

[Markus Armbruster]

Markus Armbruster  writes:

> Philippe posted "[PATCH v2 00/22] qapi: Kill 'qapi/qmp/qerror.h' for
> good" a couple of months ago.  I cherry-picked just its simplest parts
> for now.
>
> Markus Armbruster (1):
>   error: Drop superfluous #include "qapi/qmp/qerror.h"

Queued for 9.1 with PATCH 08's new error message fixed.  Thanks!

Re: [PATCH] iotests: adapt to output change for recently introduced 'detached header' field

[Kevin Wolf]

Am 16.02.2024 um 11:14 hat Fiona Ebner geschrieben:
> Failure was noticed when running the tests for the qcow2 image format.
> 
> Fixes: 0bd779e27e ("crypto: Introduce 'detached-header' field in 
> QCryptoBlockInfoLUKS")
> Signed-off-by: Fiona Ebner 

Thanks, applied to the block branch.

Kevin

[PULL 05/15] qemu-img: Fix Column Width and Improve Formatting in snapshot list

[Kevin Wolf]

From: Abhiram Tilak 

When running the command `qemu-img snapshot -l SNAPSHOT` the output of
VM_CLOCK (measures the offset between host and VM clock) cannot to
accommodate values in the order of thousands (4-digit).

This line [1] hints on the problem. Additionally, the column width for
the VM_CLOCK field was reduced from 15 to 13 spaces in commit b39847a5
in line [2], resulting in a shortage of space.

[1]:
https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L753
[2]:
https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L763

This patch restores the column width to 15 spaces and makes adjustments
to the affected iotests accordingly. Furthermore, addresses a potential
source
of confusion by removing whitespace in column headers. Example, VM CLOCK
is modified to VM_CLOCK. Additionally a '--' symbol is introduced when
ICOUNT returns no output for clarity.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2062
Fixes: b39847a50553 ("migration: introduce icount field for snapshots")
Signed-off-by: Abhiram Tilak 
Message-ID: <20240123050354.22152-2-atp@gmail.com>
[kwolf: Fixed up qemu-iotests 261 and 286]
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 block/qapi.c  | 10 ++--
 tests/qemu-iotests/176.out| 16 +++
 tests/qemu-iotests/261|  4 +-
 tests/qemu-iotests/267.out| 48 +--
 tests/qemu-iotests/286|  3 +-
 tests/qemu-iotests/286.out|  2 +-
 .../tests/qcow2-internal-snapshots.out| 14 +++---
 7 files changed, 50 insertions(+), 47 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 31183d4933..2b5793f1d9 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -742,15 +742,15 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
 char *sizing = NULL;
 
 if (!sn) {
-qemu_printf("%-10s%-17s%8s%20s%13s%11s",
-"ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
+qemu_printf("%-7s %-16s %8s %19s %15s %10s",
+"ID", "TAG", "VM_SIZE", "DATE", "VM_CLOCK", "ICOUNT");
 } else {
 g_autoptr(GDateTime) date = 
g_date_time_new_from_unix_local(sn->date_sec);
 g_autofree char *date_buf = g_date_time_format(date, "%Y-%m-%d 
%H:%M:%S");
 
 secs = sn->vm_clock_nsec / 10;
 snprintf(clock_buf, sizeof(clock_buf),
- "%02d:%02d:%02d.%03d",
+ "%04d:%02d:%02d.%03d",
  (int)(secs / 3600),
  (int)((secs / 60) % 60),
  (int)(secs % 60),
@@ -759,8 +759,10 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
 if (sn->icount != -1ULL) {
 snprintf(icount_buf, sizeof(icount_buf),
 "%"PRId64, sn->icount);
+} else {
+snprintf(icount_buf, sizeof(icount_buf), "--");
 }
-qemu_printf("%-9s %-16s %8s%20s%13s%11s",
+qemu_printf("%-7s %-16s %8s %19s %15s %10s",
 sn->id_str, sn->name,
 sizing,
 date_buf,
diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
index 45e9153ef3..9c73ef2eea 100644
--- a/tests/qemu-iotests/176.out
+++ b/tests/qemu-iotests/176.out
@@ -37,8 +37,8 @@ Offset  Length  File
 0x7ffe  0x2 TEST_DIR/t.IMGFMT.itmd
 0x8340  0x200   TEST_DIR/t.IMGFMT.itmd
 Snapshot list:
-IDTAG
-1 snap
+ID  TAG
+1   snap
 
 === Test pass snapshot.1 ===
 
@@ -78,8 +78,8 @@ Offset  Length  File
 0x7fff  0x1 TEST_DIR/t.IMGFMT
 0x8340  0x200   TEST_DIR/t.IMGFMT
 Snapshot list:
-IDTAG
-1 snap
+ID  TAG
+1   snap
 
 === Test pass snapshot.2 ===
 
@@ -119,8 +119,8 @@ Offset  Length  File
 0x7fff  0x1 TEST_DIR/t.IMGFMT
 0x8340  0x200   TEST_DIR/t.IMGFMT
 Snapshot list:
-IDTAG
-1 snap
+ID  TAG
+1   snap
 
 === Test pass snapshot.3 ===
 
@@ -157,8 +157,8 @@ Offset  Length  File
 0x7fff  0x1 TEST_DIR/t.IMGFMT
 0x8340  0x200   TEST_DIR/t.IMGFMT
 Snapshot list:
-IDTAG
-1 snap
+ID  TAG
+1   snap
 
 === Test pass bitmap.0 ===
 
diff --git a/tests/qemu-iotests/261 b/tests/qemu-iotests/261
index b73da565da..22b969d310 100755
--- a/tests/qemu-iotests/261
+++ b/tests/qemu-iotests/261
@@ -393,7 +393,7 @@ _check_test_img -r all
 
 echo
 echo "$((sn_count - 1)) snapshots should remain:"
-echo "  qemu-img info reports $(_img_info | grep -c '^ \{32\}') snapshots"
+echo "  qemu-img info reports $(_img_info | grep -c '^ \{30\}') snapshots"
 echo "  Image header reports $(peek_file_be "$TEST_IMG" 60 4) snapshots"
 
 echo
@@ -520,7 +520,7 @@ _check_test_img -r all
 
 echo
 echo '65536 snapshots should remain:'
-echo "  

[PULL 03/15] iotests: Add test for reset/AioContext switches with NBD exports

[Kevin Wolf]

This replicates the scenario in which the bug was reported.
Unfortunately this relies on actually executing a guest (so that the
firmware initialises the virtio-blk device and moves it to its
configured iothread), so this can't make use of the qtest accelerator
like most other test cases. I tried to find a different easy way to
trigger the bug, but couldn't find one.

Signed-off-by: Kevin Wolf 
Message-ID: <20240314165825.40261-3-kw...@redhat.com>
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/tests/iothreads-nbd-export | 66 +++
 .../tests/iothreads-nbd-export.out| 19 ++
 2 files changed, 85 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/iothreads-nbd-export
 create mode 100644 tests/qemu-iotests/tests/iothreads-nbd-export.out

diff --git a/tests/qemu-iotests/tests/iothreads-nbd-export 
b/tests/qemu-iotests/tests/iothreads-nbd-export
new file mode 100755
index 00..037260729c
--- /dev/null
+++ b/tests/qemu-iotests/tests/iothreads-nbd-export
@@ -0,0 +1,66 @@
+#!/usr/bin/env python3
+# group: rw quick
+#
+# Copyright (C) 2024 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+# Creator/Owner: Kevin Wolf 
+
+import time
+import qemu
+import iotests
+
+iotests.script_initialize(supported_fmts=['qcow2'],
+  supported_platforms=['linux'])
+
+with iotests.FilePath('disk1.img') as path, \
+ iotests.FilePath('nbd.sock', base_dir=iotests.sock_dir) as nbd_sock, \
+ qemu.machine.QEMUMachine(iotests.qemu_prog) as vm:
+
+img_size = '10M'
+
+iotests.log('Preparing disk...')
+iotests.qemu_img_create('-f', iotests.imgfmt, path, img_size)
+vm.add_args('-blockdev', f'file,node-name=disk-file,filename={path}')
+vm.add_args('-blockdev', 'qcow2,node-name=disk,file=disk-file')
+vm.add_args('-object', 'iothread,id=iothread0')
+vm.add_args('-device',
+'virtio-blk,drive=disk,iothread=iothread0,share-rw=on')
+
+iotests.log('Launching VM...')
+vm.add_args('-accel', 'kvm', '-accel', 'tcg')
+#vm.add_args('-accel', 'qtest')
+vm.launch()
+
+iotests.log('Exporting to NBD...')
+iotests.log(vm.qmp('nbd-server-start',
+   addr={'type': 'unix', 'data': {'path': nbd_sock}}))
+iotests.log(vm.qmp('block-export-add', type='nbd', id='exp0',
+   node_name='disk', writable=True))
+
+iotests.log('Connecting qemu-img...')
+qemu_io = iotests.QemuIoInteractive('-f', 'raw',
+f'nbd+unix:///disk?socket={nbd_sock}')
+
+iotests.log('Moving the NBD export to a different iothread...')
+for i in range(0, 10):
+iotests.log(vm.qmp('system_reset'))
+time.sleep(0.1)
+
+iotests.log('Checking that it is still alive...')
+iotests.log(vm.qmp('query-status'))
+
+qemu_io.close()
+vm.shutdown()
diff --git a/tests/qemu-iotests/tests/iothreads-nbd-export.out 
b/tests/qemu-iotests/tests/iothreads-nbd-export.out
new file mode 100644
index 00..bc514e35e5
--- /dev/null
+++ b/tests/qemu-iotests/tests/iothreads-nbd-export.out
@@ -0,0 +1,19 @@
+Preparing disk...
+Launching VM...
+Exporting to NBD...
+{"return": {}}
+{"return": {}}
+Connecting qemu-img...
+Moving the NBD export to a different iothread...
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+Checking that it is still alive...
+{"return": {"running": true, "status": "running"}}
-- 
2.44.0

Re: [PATCH v1 1/2] system/cpus: Fix pause_all_vcpus() under concurrent environment

[David Hildenbrand]

On 17.03.24 09:37, Keqian Zhu via wrote:

Both main loop thread and vCPU thread are allowed to call
pause_all_vcpus(), and in general resume_all_vcpus() is called
after it. Two issues live in pause_all_vcpus():


In general, calling pause_all_vcpus() from VCPU threads is quite dangerous.

Do we have reproducers for the cases below?



1. There is possibility that during thread T1 waits on
qemu_pause_cond with bql unlocked, other thread has called
pause_all_vcpus() and resume_all_vcpus(), then thread T1 will
stuck, because the condition all_vcpus_paused() is always false.


How can this happen?

Two threads calling pause_all_vcpus() is borderline broken, as you note.

IIRC, we should call pause_all_vcpus() only if some other mechanism 
prevents these races. For example, based on runstate changes.



Just imagine one thread calling pause_all_vcpus() while another one 
calls resume_all_vcpus(). It cannot possibly work.




2. After all_vcpus_paused() has been checked as true, we will
unlock bql to relock replay_mutex. During the bql was unlocked,
the vcpu's state may has been changed by other thread, so we
must retry.

Signed-off-by: Keqian Zhu 
---
  system/cpus.c | 29 -
  1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/system/cpus.c b/system/cpus.c
index 68d161d96b..4e41abe23e 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -571,12 +571,14 @@ static bool all_vcpus_paused(void)
  return true;
  }
  
-void pause_all_vcpus(void)

+static void request_pause_all_vcpus(void)
  {
  CPUState *cpu;
  
-qemu_clock_enable(QEMU_CLOCK_VIRTUAL, false);

  CPU_FOREACH(cpu) {
+if (cpu->stopped) {
+continue;
+}
  if (qemu_cpu_is_self(cpu)) {
  qemu_cpu_stop(cpu, true);
  } else {
@@ -584,6 +586,14 @@ void pause_all_vcpus(void)
  qemu_cpu_kick(cpu);
  }
  }
+}
+
+void pause_all_vcpus(void)
+{
+qemu_clock_enable(QEMU_CLOCK_VIRTUAL, false);
+
+retry:
+request_pause_all_vcpus();
  
  /* We need to drop the replay_lock so any vCPU threads woken up

   * can finish their replay tasks
@@ -592,14 +602,23 @@ void pause_all_vcpus(void)
  
  while (!all_vcpus_paused()) {

  qemu_cond_wait(_pause_cond, );
-CPU_FOREACH(cpu) {
-qemu_cpu_kick(cpu);
-}
+/* During we waited on qemu_pause_cond the bql was unlocked,
+ * the vcpu's state may has been changed by other thread, so
+ * we must request the pause state on all vcpus again.
+ */
+request_pause_all_vcpus();
  }
  
  bql_unlock();

  replay_mutex_lock();
  bql_lock();
+
+/* During the bql was unlocked, the vcpu's state may has been
+ * changed by other thread, so we must retry.
+ */
+if (!all_vcpus_paused()) {
+goto retry;
+}
  }
  
  void cpu_resume(CPUState *cpu)


--
Cheers,

David / dhildenb

Re: [PATCH 03/14] hw/core/machine-smp: Simplify variables' initialization in machine_parse_smp_config()

[Prasad Pandit]

Hello,

On Mon, 18 Mar 2024 at 13:23, Zhao Liu  wrote:
> Indeed, as you say, these items are initialized to 0 by default.
>
> I think, however, that the initialization is so far away from where the
> smp is currently parsed that one can't easily confirm it (thanks for
> your confirmation!).
>
> From a code readability view, the fact that we're explicitly
> initializing to 0 again here brings little overhead, but makes the code
> more readable as well as easier to maintain. I think the small redundancy
> here is worth it.
>
> Also, in other use cases people always relies on fields marked by has_*,
> and there is no (or less?) precedent for direct access to places where
> has_* is not set. I understand that this is also a habit, i.e., fields
> with a has_* of False by default are unreliable and avoid going directly
> to them.

* Ummn...okay. (I'm not fully convinced, but that's fine, I'm okay to
go with you on this.)

Thank you.
---
  - Prasad

Re: [PATCH 7/9] i386/sev: Refactor setting of reset vector and initial CPU state

[Roy Hopkins]

On Tue, 2024-03-12 at 16:12 +, Daniel P. Berrangé wrote:
> On Tue, Mar 12, 2024 at 03:45:20PM +, Roy Hopkins wrote:
> > On Fri, 2024-03-01 at 17:01 +, Daniel P. Berrangé wrote:
> > > On Tue, Feb 27, 2024 at 02:50:13PM +, Roy Hopkins wrote:
> > > > +    /*
> > > > + * Ideally we would provide the VMSA directly to kvm which
> > > > would
> > > > + * ensure that the resulting initial VMSA measurement which
> > > > is
> > > > + * calculated during KVM_SEV_LAUNCH_UPDATE_VMSA is
> > > > calculated
> > > > from
> > > > + * exactly what we provide here. Currently this is not
> > > > possible
> > > > so
> > > > + * we need to copy the parts of the VMSA structure that we
> > > > currently
> > > > + * support into the CPU state.
> > > > + */
> > > 
> > > This sounds like it is saying that the code is not honouring
> > > everything in the VMSA defiend by the IGVM file ?
> > > 
> > > If so, that is pretty awkward. The VMSA is effectively an external
> > > ABI between QEMU and the guest owner (or whatever is validating
> > > guest attestation reports for them), and thus predictability and
> > > stability of this over time is critical.
> > > 
> > > We don't want the attestation process to be dependent/variable on
> > > the particular QEMU/KVM version, because any upgrade to QEMU/KVM
> > > could then alter the effective VMSA that the guest owner sees.
> > > 
> > > We've already suffered pain in this respect not long ago when the
> > > kernel arbitrarily changed a default setting which altered the
> > > VMSA it exposed, breaking existing apps that validate attestation.
> > > 
> > > What will it take to provide the full VMSA to KVM, so that we can
> > > guarantee to the guest owner than the VMSA for the guest is going
> > > to perfectly match what their IGVM defined ?
> > > 
> > 
> > Yes, the fact that we have to copy the individual fields from the VMSA to
> > "CPUX86State" is less than ideal - a problem made worse by the fact that the
> > kernel does not allow direct control over some of the fields from userspace,
> > "sev_features" being a good example here where "SVM_SEV_FEAT_DEBUG_SWAP" is
> > unconditionally added by the kernel.
> 
> Ah yes, the SVM_SEV_FEAT_DEBUG_SWAP feature is the one I couldn't remember
> the name of in my quoted text above, that break our apps when the kernel
> suddenly set it by default (thankfully now reverted in Linux with
> 5abf6dceb066f2b02b225fd561440c98a8062681).
> 
> > The kernel VMSA is at least predictable. So, although we cannot yet allow
> > full
> > flexibility in providing a complete VMSA from QEMU and guarantee it will be
> > honoured, we could check to see if any settings conflict with those imposed
> > by
> > the kernel and exit with an error if this is the case. I chose not to
> > implement
> > for this first series but could easily add a patch to support this. The
> > problem
> > here is that it ties the version of QEMU to VMSA handling functionality in
> > the
> > kernel. Any change to the VMSA handling in the kernel would potentially
> > invalidate the checks in QEMU. The one upside here is that this will easily
> > be
> > detectable by the attestation measurement not matching the expected
> > measurement
> > of the IGVM file. But it will be difficult for the user to determine what
> > the
> > discrepancy is.
> 
> Yes, the difficulty in diagnosis is the big thing I'm worried about from
> a distro supportability POV. The DEBUG_SWAP issue caused us a bunch of
> pain and that's before CVMs are even widely used.
> 
> I agree that hardcoding checks in QEMU is pretty unpleasant, and probably
> not something that I'd want us to do. I'd want QEMU to be able to live
> query the kernel's default initial VMSA, if it were to be reporting
> differences vs the IGVM provided VMSA. I dn't think there's a way to
> do that nicely though - i only know of ftrace probes to dump it informally.
> 
> I guess if we know & document what subset of the VMSA QEMU /can/ directly
> control, that at least narrows down where to look if something does change
> or go wrong.
> 
Yes, it makes sense to document the subset that can be reliably set by QEMU,
along with any modifications made byt the kernel. Perhaps I should go one step
further and check that the VMSA does not contain any entries beyond what is
copied in "sev_apply_cpu_context()"? If any field other than those explicitly
copied by this function contain a non-zero value then an error is generated. As
you suggest this will limit the scope of any measurement differences to the
documented subset.

> > The ideal solution is to add or modify a KVM ioctl to allow the VMSA to be
> > set
> > directly, overriding the state in "CPUX86State". The current
> > KVM_SEV_LAUNCH_UPDATE_VMSA ioctl triggers the synchronisation of the VMSA
> > but
> > does not allow it to be specified directly. This could be modified for what
> > we
> > need. The SEV-SNP kernel 

[PULL 11/15] tests/qemu-iotests: Restrict test 156 to the 'file' protocol

[Kevin Wolf]

From: Thomas Huth 

The test fails completely when you try to use it with a different
protocol, e.g. with "./check -ssh -qcow2 156".
The test uses some hand-crafted JSON statements which cannot work with other
protocols, thus let's change this test to only support the 'file' protocol.

Signed-off-by: Thomas Huth 
Message-ID: <2024031508.153201-7-th...@redhat.com>
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/156 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/156 b/tests/qemu-iotests/156
index a9540bd80d..97c2d86ce5 100755
--- a/tests/qemu-iotests/156
+++ b/tests/qemu-iotests/156
@@ -50,7 +50,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.qemu
 
 _supported_fmt qcow2 qed
-_supported_proto generic
+_supported_proto file
 # Copying files around with cp does not work with external data files
 _unsupported_imgopts data_file
 
-- 
2.44.0

Re: [PATCH for 9.0 v15 02/10] trans_rvv.c.inc: set vstart = 0 in int scalar move insns

[Alistair Francis]

On Fri, Mar 15, 2024 at 3:59 AM Daniel Henrique Barboza
 wrote:
>
> trans_vmv_x_s, trans_vmv_s_x, trans_vfmv_f_s and trans_vfmv_s_f aren't
> setting vstart = 0 after execution. This is usually done by a helper in
> vector_helper.c but these functions don't use helpers.
>
> We'll set vstart after any potential 'over' brconds, and that will also
> mandate a mark_vs_dirty() too.
>
> Fixes: dedc53cbc9 ("target/riscv: rvv-1.0: integer scalar move instructions")
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Richard Henderson 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.c.inc | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index e42728990e..8c16a9f5b3 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -3373,6 +3373,8 @@ static bool trans_vmv_x_s(DisasContext *s, arg_vmv_x_s 
> *a)
>  vec_element_loadi(s, t1, a->rs2, 0, true);
>  tcg_gen_trunc_i64_tl(dest, t1);
>  gen_set_gpr(s, a->rd, dest);
> +tcg_gen_movi_tl(cpu_vstart, 0);
> +mark_vs_dirty(s);
>  return true;
>  }
>  return false;
> @@ -3399,8 +3401,9 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x 
> *a)
>  s1 = get_gpr(s, a->rs1, EXT_NONE);
>  tcg_gen_ext_tl_i64(t1, s1);
>  vec_element_storei(s, a->rd, 0, t1);
> -mark_vs_dirty(s);
>  gen_set_label(over);
> +tcg_gen_movi_tl(cpu_vstart, 0);
> +mark_vs_dirty(s);
>  return true;
>  }
>  return false;
> @@ -3427,6 +3430,8 @@ static bool trans_vfmv_f_s(DisasContext *s, 
> arg_vfmv_f_s *a)
>  }
>
>  mark_fs_dirty(s);
> +tcg_gen_movi_tl(cpu_vstart, 0);
> +mark_vs_dirty(s);
>  return true;
>  }
>  return false;
> @@ -3452,8 +3457,9 @@ static bool trans_vfmv_s_f(DisasContext *s, 
> arg_vfmv_s_f *a)
>  do_nanbox(s, t1, cpu_fpr[a->rs1]);
>
>  vec_element_storei(s, a->rd, 0, t1);
> -mark_vs_dirty(s);
>  gen_set_label(over);
> +tcg_gen_movi_tl(cpu_vstart, 0);
> +mark_vs_dirty(s);
>  return true;
>  }
>  return false;
> --
> 2.44.0
>
>

Re: [PATCH v2 3/3] qom/object_interfaces: Remove local_err in user_creatable_add_type

[Zhao Liu]

On Mon, Mar 18, 2024 at 11:32:10AM +0800, Zhenzhong Duan wrote:
> Date: Mon, 18 Mar 2024 11:32:10 +0800
> From: Zhenzhong Duan 
> Subject: [PATCH v2 3/3] qom/object_interfaces: Remove local_err in
>  user_creatable_add_type
> X-Mailer: git-send-email 2.34.1
> 
> In user_creatable_add_type, there is mixed usage of ERRP_GUARD and
> local_err. This makes error_abort not taking effect in those callee
> functions with _err passed.
> 
> Now that we already use ERRP_GUARD, remove local_err and pass errp.
> 
> Signed-off-by: Zhenzhong Duan 
> ---
>  qom/object_interfaces.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Zhao Liu 

[PATCH] ui/cocoa: Do not automatically zoom for HiDPI

[Akihiko Odaki]

Cocoa automatically zooms for a HiDPI display like Retina and makes
the display blurry. Revert the automatic zooming.

Signed-off-by: Akihiko Odaki 
---
 ui/cocoa.m | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index fa879d7dcd4b..c5b3c28000ff 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -522,7 +522,10 @@ - (void) resizeWindow
 [[self window] setContentAspectRatio:NSMakeSize(screen.width, 
screen.height)];
 
 if (!([[self window] styleMask] & NSWindowStyleMaskResizable)) {
-[[self window] setContentSize:NSMakeSize(screen.width, screen.height)];
+CGFloat width = screen.width / [[self window] backingScaleFactor];
+CGFloat height = screen.height / [[self window] backingScaleFactor];
+
+[[self window] setContentSize:NSMakeSize(width, height)];
 [[self window] center];
 } else if ([[self window] styleMask] & NSWindowStyleMaskFullScreen) {
 [[self window] setContentSize:[self screenSafeAreaSize]];
@@ -575,8 +578,8 @@ - (void) updateUIInfoLocked
 
 info.xoff = 0;
 info.yoff = 0;
-info.width = frameSize.width;
-info.height = frameSize.height;
+info.width = frameSize.width * [[self window] backingScaleFactor];
+info.height = frameSize.height * [[self window] backingScaleFactor];
 
 dpy_set_ui_info(dcl.con, , TRUE);
 }

---
base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
change-id: 20240318-zoom-df4d6834e56b

Best regards,
-- 
Akihiko Odaki 

[RFC PATCH v8 00/23] target/arm: Implement FEAT_NMI and FEAT_GICv3_NMI

[Jinjie Ruan via]

This patch set implements FEAT_NMI and FEAT_GICv3_NMI for armv8. These
introduce support for a new category of interrupts in the architecture
which we can use to provide NMI like functionality.

There are two modes for using this FEAT_NMI. When PSTATE.ALLINT or
PSTATE.SP & SCTLR_ELx.SCTLR_SPINTMASK is set, any entry to ELx causes all
interrupts including those with superpriority to be masked on entry to ELn
until the mask is explicitly removed by software or hardware. PSTATE.ALLINT
can be managed by software using the new register control ALLINT.ALLINT.
Independent controls are provided for this feature at each EL, usage at EL1
should not disrupt EL2 or EL3.

I have tested it with the following linux patches which try to support
FEAT_NMI in linux kernel:


https://lore.kernel.org/linux-arm-kernel/Y4sH5qX5bK9xfEBp@lpieralisi/T/#mb4ba4a2c045bf72c10c2202c1dd1b82d3240dc88

In the test, SGI, PPI and SPI interrupts can all be set to have super priority
to be converted to a hardware NMI interrupt. The SGI is tested with kernel
IPI as NMI framework, softlockup, hardlockup and kgdb test cases, and the PPI
interrupt is tested with "perf top" command with hardware NMI enabled, and
the SPI interrupt is tested with a custom test module, in which NMI interrupts
can be received and sent normally.

And the Virtual NMI(VNMI) SGI, PPI and SPI interrupt has also been tested
by accessing the GICD_INMIR, GICR_INMIR0, GICR_ISPENDR0 and GICD_ISPENDR
registers with devmem command.

 +-+
 |   Distributor   |
 +-+
 SPI |  NMI |  NMI
\/\/
++ +---+
| Redist | | Redist|
++ +---+
SGI  | NMI PPI | NMI
\/\/
  +-+ +---+
  |CPU Interface|   ...   | CPU Interface |
  +-+ +---+
   | NMI | NMI
  \/\/
+-+   +-+
|  PE |   |  PE |
+-+   +-+

Changes in v8:
- Fix the rcu stall after sending a VNMI in qemu VM.
- Fix an unexpected interrupt bug when sending VNMI by running qemu VM.
- Test the VNMI interrupt.
- Update the commit message.
- Add Reviewed-by.

Changes in v7:
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Reorder the irqbetter() code for clarity.
- Eliminate the has_superprio local variable for gicv3_get_priority().
- false -> cs->hpplpi.superprio in gicv3_redist_update_noirqset().
- 0x0 -> false in arm_gicv3_common_reset_hold().
- Clear superprio in several places for hppi, hpplpi and hppvlpi.
- Add Reviewed-by.

Changes in v6:
- Fix DISAS_TOO_MANY to DISAS_UPDATE_EXIT for ALLINT MSR (immediate).
- Verify that HCR_EL2.VF is set before checking VFNMI.
- env->cp15.hcr_el2 -> arm_hcr_el2_eff().
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Not combine VFNMI with CPU_INTERRUPT_VNMI.
- Implement icv_nmiar1_read().
- Put the "extract superprio info" code into gicv3_get_priority().
- Update the comment in irqbetter().
- Reset the cs->hppi.superprio to 0x0.
- Set hppi.superprio to false for LPI.
- Add Reviewed-by.

Changes in v5:
- Remove the comment for ALLINT in cpu.h.
- Merge allint_check() to msr_i_allint to clear the ALLINT MSR (immediate)
  implementation.
- Rename msr_i_allint() to msr_set_allint_el1() to make it clearer.
- Drop the & 1 in trans_MSR_i_ALLINT().
- Add Reviewed-by.

Changes in v4:
- Handle VNMI within the CPU and the GIC.
- Keep PSTATE.ALLINT in env->pstate but not env->allint.
- Fix the ALLINT MSR (immediate) decodetree implementation.
- Accept NMI unconditionally for arm_cpu_has_work() but add comment.
- Improve nmi mask in arm_excp_unmasked().
- Make the GICR_INMIR0 and GICD_INMIR implementation more clearer.
- Improve ICC_NMIAR1_EL1 implementation
- Extract gicv3_get_priority() to avoid priority code repetition.
- Add Reviewed-by.

Changes in v3:
- Remove the FIQ NMI.
- Adjust the patches Sequence.
- Reomve the patch "Set pstate.ALLINT in arm_cpu_reset_hold".
- Check whether support FEAT_NMI and FEAT_GICv3 for FEAT_GICv3_NMI.
- With CPU_INTERRUPT_NMI, both CPSR_I and ISR_IS must be set.
- Not include NMI logic when FEAT_NMI or SCTLR_ELx.NMI not enabled.
- Refator nmi mask in arm_excp_unmasked().
- Add VNMI definitions, add HCRX_VINMI and HCRX_VFNMI support in HCRX_EL2.
- Add Reviewed-by and Acked-by.
- Update the commit message.

Changes in v2:
- Break up the patches so that each one does only one thing.
- Remove the command line option and just implement it in "max" cpu.

Jinjie Ruan (23):
  target/arm: 

[RFC PATCH v8 21/23] hw/intc/arm_gicv3: Report the VNMI interrupt

[Jinjie Ruan via]

In vCPU Interface, if the vIRQ has the superpriority property, report
vNMI to the corresponding vPE.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v6:
- Add Reviewed-by.
---
 hw/intc/arm_gicv3_cpuif.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 29142fcf8d..0bea6cb020 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -465,6 +465,7 @@ void gicv3_cpuif_virt_irq_fiq_update(GICv3CPUState *cs)
 int idx;
 int irqlevel = 0;
 int fiqlevel = 0;
+int nmilevel = 0;
 
 idx = hppvi_index(cs);
 trace_gicv3_cpuif_virt_update(gicv3_redist_affid(cs), idx,
@@ -482,9 +483,17 @@ void gicv3_cpuif_virt_irq_fiq_update(GICv3CPUState *cs)
 uint64_t lr = cs->ich_lr_el2[idx];
 
 if (icv_hppi_can_preempt(cs, lr)) {
-/* Virtual interrupts are simple: G0 are always FIQ, and G1 IRQ */
+/*
+ * Virtual interrupts are simple: G0 are always FIQ, and G1 are
+ * IRQ or NMI which depends on the ICH_LR_EL2.NMI to have
+ * non-maskable property.
+ */
 if (lr & ICH_LR_EL2_GROUP) {
-irqlevel = 1;
+if (cs->gic->nmi_support && (lr & ICH_LR_EL2_NMI)) {
+nmilevel = 1;
+} else {
+irqlevel = 1;
+}
 } else {
 fiqlevel = 1;
 }
@@ -494,6 +503,7 @@ void gicv3_cpuif_virt_irq_fiq_update(GICv3CPUState *cs)
 trace_gicv3_cpuif_virt_set_irqs(gicv3_redist_affid(cs), fiqlevel, 
irqlevel);
 qemu_set_irq(cs->parent_vfiq, fiqlevel);
 qemu_set_irq(cs->parent_virq, irqlevel);
+qemu_set_irq(cs->parent_vnmi, nmilevel);
 }
 
 static void gicv3_cpuif_virt_update(GICv3CPUState *cs)
-- 
2.34.1

[RFC PATCH v8 06/23] target/arm: Add support for Non-maskable Interrupt

[Jinjie Ruan via]

This only implements the external delivery method via the GICv3.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v8:
- Fix the rcu stall after sending a VNMI in qemu VM.
v7:
- Add Reviewed-by.
v6:
- env->cp15.hcr_el2 -> arm_hcr_el2_eff().
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Not include VF && VFNMI in CPU_INTERRUPT_VNMI.
v4:
- Accept NMI unconditionally for arm_cpu_has_work() but add comment.
- Change from & to && for EXCP_IRQ or EXCP_FIQ.
- Refator nmi mask in arm_excp_unmasked().
- Also handle VNMI in arm_cpu_exec_interrupt() and arm_cpu_set_irq().
- Rename virtual to Virtual.
v3:
- Not include CPU_INTERRUPT_NMI when FEAT_NMI not enabled
- Add ARM_CPU_VNMI.
- Refator nmi mask in arm_excp_unmasked().
- Test SCTLR_ELx.NMI for ALLINT mask for NMI.
---
 target/arm/cpu-qom.h   |  4 +-
 target/arm/cpu.c   | 85 +++---
 target/arm/cpu.h   |  4 ++
 target/arm/helper.c|  2 +
 target/arm/internals.h |  9 +
 5 files changed, 97 insertions(+), 7 deletions(-)

diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 8e032691db..e0c9e18036 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -36,11 +36,13 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
 #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
 #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
 
-/* Meanings of the ARMCPU object's four inbound GPIO lines */
+/* Meanings of the ARMCPU object's six inbound GPIO lines */
 #define ARM_CPU_IRQ 0
 #define ARM_CPU_FIQ 1
 #define ARM_CPU_VIRQ 2
 #define ARM_CPU_VFIQ 3
+#define ARM_CPU_NMI 4
+#define ARM_CPU_VNMI 5
 
 /* For M profile, some registers are banked secure vs non-secure;
  * these are represented as a 2-element array where the first element
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index ab8d007a86..f8d931a917 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -122,6 +122,13 @@ void arm_restore_state_to_opc(CPUState *cs,
 }
 #endif /* CONFIG_TCG */
 
+/*
+ * With SCTLR_ELx.NMI == 0, IRQ with Superpriority is masked identically with
+ * IRQ without Superpriority. Moreover, if the GIC is configured so that
+ * FEAT_GICv3_NMI is only set if FEAT_NMI is set, then we won't ever see
+ * CPU_INTERRUPT_*NMI anyway. So we might as well accept NMI here
+ * unconditionally.
+ */
 static bool arm_cpu_has_work(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
@@ -129,6 +136,7 @@ static bool arm_cpu_has_work(CPUState *cs)
 return (cpu->power_state != PSCI_OFF)
 && cs->interrupt_request &
 (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
+ | CPU_INTERRUPT_NMI | CPU_INTERRUPT_VNMI
  | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ | CPU_INTERRUPT_VSERR
  | CPU_INTERRUPT_EXITTB);
 }
@@ -668,6 +676,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned 
int excp_idx,
 CPUARMState *env = cpu_env(cs);
 bool pstate_unmasked;
 bool unmasked = false;
+bool allIntMask = false;
 
 /*
  * Don't take exceptions if they target a lower EL.
@@ -678,13 +687,31 @@ static inline bool arm_excp_unmasked(CPUState *cs, 
unsigned int excp_idx,
 return false;
 }
 
+if (cpu_isar_feature(aa64_nmi, env_archcpu(env)) &&
+env->cp15.sctlr_el[target_el] & SCTLR_NMI && cur_el == target_el) {
+allIntMask = env->pstate & PSTATE_ALLINT ||
+ ((env->cp15.sctlr_el[target_el] & SCTLR_SPINTMASK) &&
+  (env->pstate & PSTATE_SP));
+}
+
 switch (excp_idx) {
+case EXCP_NMI:
+pstate_unmasked = !allIntMask;
+break;
+
+case EXCP_VNMI:
+if ((!(hcr_el2 & HCR_IMO) && !(hcr_el2 & HCR_FMO)) ||
+ (hcr_el2 & HCR_TGE)) {
+/* VNMIs(VIRQs or VFIQs) are only taken when hypervized.  */
+return false;
+}
+return !allIntMask;
 case EXCP_FIQ:
-pstate_unmasked = !(env->daif & PSTATE_F);
+pstate_unmasked = (!(env->daif & PSTATE_F)) && (!allIntMask);
 break;
 
 case EXCP_IRQ:
-pstate_unmasked = !(env->daif & PSTATE_I);
+pstate_unmasked = (!(env->daif & PSTATE_I)) && (!allIntMask);
 break;
 
 case EXCP_VFIQ:
@@ -692,13 +719,13 @@ static inline bool arm_excp_unmasked(CPUState *cs, 
unsigned int excp_idx,
 /* VFIQs are only taken when hypervized.  */
 return false;
 }
-return !(env->daif & PSTATE_F);
+return !(env->daif & PSTATE_F) && (!allIntMask);
 case EXCP_VIRQ:
 if (!(hcr_el2 & HCR_IMO) || (hcr_el2 & HCR_TGE)) {
 /* VIRQs are only taken when hypervized.  */
 return false;
 }
-return !(env->daif & PSTATE_I);
+return !(env->daif & PSTATE_I) && (!allIntMask);
 case EXCP_VSERR:
 if (!(hcr_el2 & HCR_AMO) || (hcr_el2 & HCR_TGE)) {
 /* VIRQs are only taken when hypervized.  */
@@ -804,6 +831,24 @@ static bool arm_cpu_exec_interrupt(CPUState *cs, int 

[RFC PATCH v8 07/23] target/arm: Add support for NMI in arm_phys_excp_target_el()

[Jinjie Ruan via]

According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
with superpriority is always IRQ, never FIQ, so handle NMI same as IRQ in
arm_phys_excp_target_el().

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v4:
- Add Reviewed-by.
v3:
- Remove nmi_is_irq flag in CPUARMState.
- Handle NMI same as IRQ in arm_phys_excp_target_el().
---
 target/arm/helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 875a7fa8da..5c637f3413 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10735,6 +10735,7 @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t 
excp_idx,
 hcr_el2 = arm_hcr_el2_eff(env);
 switch (excp_idx) {
 case EXCP_IRQ:
+case EXCP_NMI:
 scr = ((env->cp15.scr_el3 & SCR_IRQ) == SCR_IRQ);
 hcr = hcr_el2 & HCR_IMO;
 break;
-- 
2.34.1

[RFC PATCH v8 13/23] hw/intc/arm_gicv3: Add irq superpriority information

[Jinjie Ruan via]

A SPI, PPI or SGI interrupt can have a superpriority property. So
maintain superpriority information in PendingIrq and GICR/GICD.

Signed-off-by: Jinjie Ruan 
Acked-by: Richard Henderson 
---
v3:
- Place this ahead of implement GICR_INMIR.
- Add Acked-by.
---
 include/hw/intc/arm_gicv3_common.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/hw/intc/arm_gicv3_common.h 
b/include/hw/intc/arm_gicv3_common.h
index 7324c7d983..df4380141d 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -146,6 +146,7 @@ typedef struct {
 int irq;
 uint8_t prio;
 int grp;
+bool superprio;
 } PendingIrq;
 
 struct GICv3CPUState {
@@ -172,6 +173,7 @@ struct GICv3CPUState {
 uint32_t gicr_ienabler0;
 uint32_t gicr_ipendr0;
 uint32_t gicr_iactiver0;
+uint32_t gicr_isuperprio;
 uint32_t edge_trigger; /* ICFGR0 and ICFGR1 even bits */
 uint32_t gicr_igrpmodr0;
 uint32_t gicr_nsacr;
@@ -274,6 +276,7 @@ struct GICv3State {
 GIC_DECLARE_BITMAP(active);   /* GICD_ISACTIVER */
 GIC_DECLARE_BITMAP(level);/* Current level */
 GIC_DECLARE_BITMAP(edge_trigger); /* GICD_ICFGR even bits */
+GIC_DECLARE_BITMAP(superprio);/* GICD_INMIR */
 uint8_t gicd_ipriority[GICV3_MAXIRQ];
 uint64_t gicd_irouter[GICV3_MAXIRQ];
 /* Cached information: pointer to the cpu i/f for the CPUs specified
@@ -313,6 +316,7 @@ GICV3_BITMAP_ACCESSORS(pending)
 GICV3_BITMAP_ACCESSORS(active)
 GICV3_BITMAP_ACCESSORS(level)
 GICV3_BITMAP_ACCESSORS(edge_trigger)
+GICV3_BITMAP_ACCESSORS(superprio)
 
 #define TYPE_ARM_GICV3_COMMON "arm-gicv3-common"
 typedef struct ARMGICv3CommonClass ARMGICv3CommonClass;
-- 
2.34.1

Re: [PATCH 03/22] tcg: Pass function pointer to tcg_gen_call*

[Alex Bennée]

Richard Henderson  writes:

> For normal helpers, read the function pointer from the
> structure earlier.  For plugins, this will allow the
> function pointer to come from elsewhere.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 02/22] tcg: Make tcg/helper-info.h self-contained

[Alex Bennée]

Richard Henderson  writes:

> Move MAX_CALL_IARGS from tcg.h and include for
> the define of TCG_TARGET_REG_BITS.
>
> Signed-off-by: Richard Henderson 

This may have broken TCI:

../tcg/tci.c: In function 'tcg_qemu_tb_exec':
../tcg/tci.c:391:34: error: 'MAX_CALL_IARGS' undeclared (first use in this 
function)
  391 | void *call_slots[MAX_CALL_IARGS];
  |  ^~
../tcg/tci.c:391:34: note: each undeclared identifier is reported only once for 
each function it appears in
../tcg/tci.c:391:23: error: unused variable 'call_slots' 
[-Werror=unused-variable]
  391 | void *call_slots[MAX_CALL_IARGS];
  |   ^~
cc1: all warnings being treated as errors

the gift that keeps on giving ;-)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 02/22] tcg: Make tcg/helper-info.h self-contained

[Alex Bennée]

Richard Henderson  writes:

> Move MAX_CALL_IARGS from tcg.h and include for
> the define of TCG_TARGET_REG_BITS.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH] hw/virtio: Add support for VDPA network simulation devices

[Michael S. Tsirkin]

On Thu, Mar 14, 2024 at 11:24:33AM +0800, Jason Wang wrote:
> On Thu, Mar 14, 2024 at 3:52 AM Michael S. Tsirkin  wrote:
> >
> > On Wed, Mar 13, 2024 at 07:51:08PM +0100, Thomas Weißschuh wrote:
> > > On 2024-02-21 15:38:02+0800, Hao Chen wrote:
> > > > This patch adds support for VDPA network simulation devices.
> > > > The device is developed based on virtio-net and tap backend,
> > > > and supports hardware live migration function.
> > > >
> > > > For more details, please refer to "docs/system/devices/vdpa-net.rst"
> > > >
> > > > Signed-off-by: Hao Chen 
> > > > ---
> > > >  MAINTAINERS |   5 +
> > > >  docs/system/device-emulation.rst|   1 +
> > > >  docs/system/devices/vdpa-net.rst| 121 +
> > > >  hw/net/virtio-net.c |  16 ++
> > > >  hw/virtio/virtio-pci.c  | 189 +++-
> 
> I think those modifications should belong to a separate file as it
> might conflict with virito features in the future.
> 
> > > >  hw/virtio/virtio.c  |  39 
> > > >  include/hw/virtio/virtio-pci.h  |   5 +
> > > >  include/hw/virtio/virtio.h  |  19 ++
> > > >  include/standard-headers/linux/virtio_pci.h |   7 +
> > > >  9 files changed, 399 insertions(+), 3 deletions(-)
> > > >  create mode 100644 docs/system/devices/vdpa-net.rst
> > >
> > > [..]
> > >
> > > > diff --git a/include/standard-headers/linux/virtio_pci.h 
> > > > b/include/standard-headers/linux/virtio_pci.h
> > > > index b7fdfd0668..fb5391cef6 100644
> > > > --- a/include/standard-headers/linux/virtio_pci.h
> > > > +++ b/include/standard-headers/linux/virtio_pci.h
> > > > @@ -216,6 +216,13 @@ struct virtio_pci_cfg_cap {
> > > >  #define VIRTIO_PCI_COMMON_Q_NDATA  56
> > > >  #define VIRTIO_PCI_COMMON_Q_RESET  58
> > > >
> > > > +#define LM_LOGGING_CTRL 0
> > > > +#define LM_BASE_ADDR_LOW4
> > > > +#define LM_BASE_ADDR_HIGH   8
> > > > +#define LM_END_ADDR_LOW 12
> > > > +#define LM_END_ADDR_HIGH16
> > > > +#define LM_VRING_STATE_OFFSET   0x20
> > >
> > > These changes are not in upstream Linux and will be undone by
> > > ./scripts/update-linux-headers.sh.
> > >
> > > Are they intentionally in this header?
> >
> >
> > Good point. Pls move.
> 
> Right and this part, it's not a part of standard virtio.
> 
> Thanks

I'm thinking of reverting this patch unless there's a resolution
soon, and reapplying later after the release.


> >
> > > > +
> > > >  #endif /* VIRTIO_PCI_NO_MODERN */
> > > >
> > > >  #endif
> >

Re: [PATCH for-9.0] mirror: Don't call job_pause_point() under graph lock

[Stefan Hajnoczi]

On Mon, Mar 18, 2024 at 12:37:19PM +0100, Kevin Wolf wrote:
> Am 14.03.2024 um 15:29 hat Stefan Hajnoczi geschrieben:
> > On Wed, Mar 13, 2024 at 04:30:00PM +0100, Kevin Wolf wrote:
> > > Calling job_pause_point() while holding the graph reader lock
> > > potentially results in a deadlock: bdrv_graph_wrlock() first drains
> > > everything, including the mirror job, which pauses it. The job is only
> > > unpaused at the end of the drain section, which is when the graph writer
> > > lock has been successfully taken. However, if the job happens to be
> > > paused at a pause point where it still holds the reader lock, the writer
> > > lock can't be taken as long as the job is still paused.
> > > 
> > > Mark job_pause_point() as GRAPH_UNLOCKED and fix mirror accordingly.
> > > 
> > > Cc: qemu-sta...@nongnu.org
> > > Buglink: https://issues.redhat.com/browse/RHEL-28125
> > > Fixes: 004915a96a7a40e942ac85e6d22518cbcd283506
> > > Signed-off-by: Kevin Wolf 
> > > ---
> > >  include/qemu/job.h |  2 +-
> > >  block/mirror.c | 10 ++
> > >  2 files changed, 7 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/include/qemu/job.h b/include/qemu/job.h
> > > index 9ea98b5927..2b873f2576 100644
> > > --- a/include/qemu/job.h
> > > +++ b/include/qemu/job.h
> > > @@ -483,7 +483,7 @@ void job_enter(Job *job);
> > >   *
> > >   * Called with job_mutex *not* held.
> > >   */
> > > -void coroutine_fn job_pause_point(Job *job);
> > > +void coroutine_fn GRAPH_UNLOCKED job_pause_point(Job *job);
> > >  
> > >  /**
> > >   * @job: The job that calls the function.
> > > diff --git a/block/mirror.c b/block/mirror.c
> > > index 5145eb53e1..1bdce3b657 100644
> > > --- a/block/mirror.c
> > > +++ b/block/mirror.c
> > > @@ -479,9 +479,9 @@ static unsigned mirror_perform(MirrorBlockJob *s, 
> > > int64_t offset,
> > >  return bytes_handled;
> > >  }
> > >  
> > > -static void coroutine_fn GRAPH_RDLOCK mirror_iteration(MirrorBlockJob *s)
> > > +static void coroutine_fn GRAPH_UNLOCKED mirror_iteration(MirrorBlockJob 
> > > *s)
> > >  {
> > > -BlockDriverState *source = s->mirror_top_bs->backing->bs;
> > > +BlockDriverState *source;
> > >  MirrorOp *pseudo_op;
> > >  int64_t offset;
> > >  /* At least the first dirty chunk is mirrored in one iteration. */
> > > @@ -489,6 +489,10 @@ static void coroutine_fn GRAPH_RDLOCK 
> > > mirror_iteration(MirrorBlockJob *s)
> > >  bool write_zeroes_ok = 
> > > bdrv_can_write_zeroes_with_unmap(blk_bs(s->target));
> > >  int max_io_bytes = MAX(s->buf_size / MAX_IN_FLIGHT, MAX_IO_BYTES);
> > >  
> > > +bdrv_graph_co_rdlock();
> > > +source = s->mirror_top_bs->backing->bs;
> > 
> > Is bdrv_ref(source) needed here so that source cannot go away if someone
> > else write locks the graph and removes it? Or maybe something else
> > protects against that. Either way, please add a comment that explains
> > why this is safe.
> 
> We didn't even get to looking at this level of detail with the graph
> locking work. We probably should, but this is not the only place in
> mirror we need to look at then. Commit 004915a9 just took the lazy path
> of taking the lock for the whole function, and it turns out that this
> was wrong and causes deadlocks, so I'm reverting it and replacing it
> with what other parts of the code do - the minimal thing to let it
> compile.
> 
> I think we already own a reference, we do a block_job_add_bdrv() in
> mirror_start_job(). But once it changes, we have a reference to the
> wrong node. So it looks to me that mirror has a problem with a changing
> source node that is more fundamental than graph locking in one specific
> function because it stores BDS pointers in its state.
> 
> Active commit already freezes the backing chain between mirror_top_bs
> and target, maybe other mirror jobs need to freeze the link between
> mirror_top_bs and source at least.
> 
> So I agree that it might be worth looking into this more, but I consider
> it unrelated to this patch. We just go back to the state in which it has
> always been before 8.2 (which might contain a latent bug that apparently
> never triggered in practice) to fix a regression that we do see in
> practice.
> 
> Kevin

Okay:

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

[PULL 09/15] tests/qemu-iotests: Restrict test 130 to the 'file' protocol

[Kevin Wolf]

From: Thomas Huth 

Using "-drive ...,backing.file.filename=..." only works with the
file protocol, but not with URIs, so mark this test accordingly.

Signed-off-by: Thomas Huth 
Message-ID: <2024031508.153201-5-th...@redhat.com>
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/130 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/130 b/tests/qemu-iotests/130
index 7257f09677..7af85d20a8 100755
--- a/tests/qemu-iotests/130
+++ b/tests/qemu-iotests/130
@@ -42,7 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.qemu
 
 _supported_fmt qcow2
-_supported_proto generic
+_supported_proto file
 _supported_os Linux
 # We are going to use lazy-refcounts
 _unsupported_imgopts 'compat=0.10'
-- 
2.44.0

Re: [PATCH for-9.0 0/2] nbd: Fix server crash on reset with iothreads

[Stefan Hajnoczi]

On Thu, Mar 14, 2024 at 05:58:23PM +0100, Kevin Wolf wrote:
> Kevin Wolf (2):
>   nbd/server: Fix race in draining the export
>   iotests: Add test for reset/AioContext switches with NBD exports
> 
>  nbd/server.c  | 15 ++---
>  tests/qemu-iotests/tests/iothreads-nbd-export | 66 +++
>  .../tests/iothreads-nbd-export.out| 19 ++
>  3 files changed, 92 insertions(+), 8 deletions(-)
>  create mode 100755 tests/qemu-iotests/tests/iothreads-nbd-export
>  create mode 100644 tests/qemu-iotests/tests/iothreads-nbd-export.out
> 
> -- 
> 2.44.0
> 

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types

[Zhao Liu]

Hi Philippe,

On Fri, Mar 15, 2024 at 02:08:54PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:54 +0100
> From: Philippe Mathieu-Daudé 
> Subject: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new
>  I386_CPU / X86_64_CPU types
> X-Mailer: git-send-email 2.41.0
> 
> "target/foo/cpu-qom.h" can not use any target specific definitions.
> 
> Currently "target/i386/cpu-qom.h" defines TYPE_X86_CPU depending
> on the i386/x86_64 build type. This doesn't scale in a heterogeneous
> context where we need to access both types concurrently.

Does this mean that there would be a TCG case contains both 64-bit and
32-bit i386 core? ;-)

> In order to do that, introduce the new I386_CPU / X86_64_CPU
> types, both inheriting a common TYPE_X86_CPU base type.
> 
> Keep the current "base" and "max" CPU types as 32 or 64-bit,
> depending on the binary built.
> 
> Adapt the cpu-plug-test, since the 'base' architecture is now
> common to both 32/64-bit x86 targets.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Acked-by: Richard Henderson 
> ---
>  target/i386/cpu-qom.h   | 16 ++--
>  target/i386/cpu.c   | 20 ++--
>  tests/qtest/cpu-plug-test.c |  2 +-
>  3 files changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/target/i386/cpu-qom.h b/target/i386/cpu-qom.h
> index d4e216d000..de28d7ea20 100644
> --- a/target/i386/cpu-qom.h
> +++ b/target/i386/cpu-qom.h
> @@ -1,5 +1,5 @@
>  /*
> - * QEMU x86 CPU
> + * QEMU x86 CPU QOM header (target agnostic)
>   *
>   * Copyright (c) 2012 SUSE LINUX Products GmbH
>   *
> @@ -22,14 +22,18 @@
>  
>  #include "hw/core/cpu.h"
>  
> -#ifdef TARGET_X86_64
> -#define TYPE_X86_CPU "x86_64-cpu"
> -#else
> -#define TYPE_X86_CPU "i386-cpu"
> -#endif
> +#define TYPE_X86_CPU"x86-cpu"
> +#define TYPE_I386_CPU   "i386-cpu"
> +#define TYPE_X86_64_CPU "x86_64-cpu"
>  
>  OBJECT_DECLARE_CPU_TYPE(X86CPU, X86CPUClass, X86_CPU)
>  
> +OBJECT_DECLARE_CPU_TYPE(I386CPU, X86CPUClass, I386_CPU)
> +OBJECT_DECLARE_CPU_TYPE(X86_64CPU, X86CPUClass, X86_64_CPU)
> +
> +#define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
> +#define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
> +

X86_CPU_TYPE_NAME seems to be duplicated because the following line is
the existing X86_CPU_TYPE_NAME definition.

>  #define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
>  #define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
>  
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ebf555f50f..07f64c1ea5 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8057,12 +8057,28 @@ static const TypeInfo x86_cpu_types[] = {
>  .class_size = sizeof(X86CPUClass),
>  .class_init = x86_cpu_common_class_init,
>  }, {
> -.name   = X86_CPU_TYPE_NAME("base"),
> +.name   = TYPE_I386_CPU,
>  .parent = TYPE_X86_CPU,
> +.abstract   = true,
> +}, {
> +.name   = TYPE_X86_64_CPU,
> +.parent = TYPE_X86_CPU,
> +.abstract   = true,
> +}, {

Should TYPE_I386_CPU/TYPE_X86_64_CPU be also wrapped with TARGET_X86_64?

Otherwise, we would keep the 32-bit CPU type definition of TYPE_I386_CPU
in the 64-bit case.

> +.name   = X86_CPU_TYPE_NAME("base"),
> +#ifdef TARGET_X86_64
> +.parent = TYPE_X86_64_CPU,
> +#else
> +.parent = TYPE_I386_CPU,
> +#endif
>  .class_init = x86_cpu_base_class_init,
>  }, {
>  .name   = X86_CPU_TYPE_NAME("max"),
> -.parent = TYPE_X86_CPU,
> +#ifdef TARGET_X86_64
> +.parent = TYPE_X86_64_CPU,
> +#else
> +.parent = TYPE_I386_CPU,
> +#endif
>  .instance_init  = max_x86_cpu_initfn,
>  .class_init = max_x86_cpu_class_init,
>  }

[RFC PATCH v8 17/23] hw/intc/arm_gicv3: Add NMI handling CPU interface registers

[Jinjie Ruan via]

Add the NMIAR CPU interface registers which deal with acknowledging NMI.

When introduce NMI interrupt, there are some updates to the semantics for the
register ICC_IAR1_EL1 and ICC_HPPIR1_EL1. For ICC_IAR1_EL1 register, it
should return 1022 if the intid has super priority. And for ICC_NMIAR1_EL1
register, it should return 1023 if the intid do not have super priority.
Howerever, these are not necessary for ICC_HPPIR1_EL1 register.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v7:
- Add Reviewed-by.
v4:
- Define ICC_NMIAR1_EL1 only if FEAT_GICv3_NMI is implemented.
- Check sctrl_elx.SCTLR_NMI to return 1022 for icc_iar1_read().
- Add gicv3_icc_nmiar1_read() trace event.
- Do not check icc_hppi_can_preempt() for icc_nmiar1_read().
- Add icv_nmiar1_read() and call it when EL2Enabled() and HCR_EL2.IMO == '1'
---
 hw/intc/arm_gicv3_cpuif.c | 59 +--
 hw/intc/gicv3_internal.h  |  1 +
 hw/intc/trace-events  |  1 +
 3 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index e1a60d8c15..df82a413c6 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -795,6 +795,13 @@ static uint64_t icv_iar_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 return intid;
 }
 
+static uint64_t icv_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+/* todo */
+uint64_t intid = INTID_SPURIOUS;
+return intid;
+}
+
 static uint32_t icc_fullprio_mask(GICv3CPUState *cs)
 {
 /*
@@ -1097,7 +1104,8 @@ static uint64_t icc_hppir0_value(GICv3CPUState *cs, 
CPUARMState *env)
 return cs->hppi.irq;
 }
 
-static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env)
+static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env,
+ bool is_nmi, bool is_hppi)
 {
 /* Return the highest priority pending interrupt register value
  * for group 1.
@@ -1108,6 +1116,19 @@ static uint64_t icc_hppir1_value(GICv3CPUState *cs, 
CPUARMState *env)
 return INTID_SPURIOUS;
 }
 
+if (!is_hppi) {
+int el = arm_current_el(env);
+
+if (is_nmi && (!cs->hppi.superprio)) {
+return INTID_SPURIOUS;
+}
+
+if ((!is_nmi) && cs->hppi.superprio
+&& env->cp15.sctlr_el[el] & SCTLR_NMI) {
+return INTID_NMI;
+}
+}
+
 /* Check whether we can return the interrupt or if we should return
  * a special identifier, as per the CheckGroup1ForSpecialIdentifiers
  * pseudocode. (We can simplify a little because for us ICC_SRE_EL1.RM
@@ -1168,7 +1189,7 @@ static uint64_t icc_iar1_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 if (!icc_hppi_can_preempt(cs)) {
 intid = INTID_SPURIOUS;
 } else {
-intid = icc_hppir1_value(cs, env);
+intid = icc_hppir1_value(cs, env, false, false);
 }
 
 if (!gicv3_intid_is_special(intid)) {
@@ -1179,6 +1200,25 @@ static uint64_t icc_iar1_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 return intid;
 }
 
+static uint64_t icc_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+GICv3CPUState *cs = icc_cs_from_env(env);
+uint64_t intid;
+
+if (icv_access(env, HCR_IMO)) {
+return icv_nmiar1_read(env, ri);
+}
+
+intid = icc_hppir1_value(cs, env, true, false);
+
+if (!gicv3_intid_is_special(intid)) {
+icc_activate_irq(cs, intid);
+}
+
+trace_gicv3_icc_nmiar1_read(gicv3_redist_affid(cs), intid);
+return intid;
+}
+
 static void icc_drop_prio(GICv3CPUState *cs, int grp)
 {
 /* Drop the priority of the currently active interrupt in
@@ -1555,7 +1595,7 @@ static uint64_t icc_hppir1_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 return icv_hppir_read(env, ri);
 }
 
-value = icc_hppir1_value(cs, env);
+value = icc_hppir1_value(cs, env, false, true);
 trace_gicv3_icc_hppir1_read(gicv3_redist_affid(cs), value);
 return value;
 }
@@ -2482,6 +2522,15 @@ static const ARMCPRegInfo 
gicv3_cpuif_icc_apxr23_reginfo[] = {
 },
 };
 
+static const ARMCPRegInfo gicv3_cpuif_gicv3_nmi_reginfo[] = {
+{ .name = "ICC_NMIAR1_EL1", .state = ARM_CP_STATE_BOTH,
+  .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 9, .opc2 = 5,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .access = PL1_R, .accessfn = gicv3_irq_access,
+  .readfn = icc_nmiar1_read,
+},
+};
+
 static uint64_t ich_ap_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
 GICv3CPUState *cs = icc_cs_from_env(env);
@@ -2838,6 +2887,10 @@ void gicv3_init_cpuif(GICv3State *s)
  */
 define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
 
+if (s->nmi_support) {
+define_arm_cp_regs(cpu, gicv3_cpuif_gicv3_nmi_reginfo);
+}
+
 /*
  * The CPU implementation specifies the number of supported
  * bits of physical priority. For backwards compatibility
diff --git a/hw/intc/gicv3_internal.h 

[RFC PATCH v8 19/23] hw/intc/arm_gicv3: Implement NMI interrupt prioirty

[Jinjie Ruan via]

If GICD_CTLR_DS bit is zero and the NMI is non-secure, the NMI prioirty
is higher than 0x80, otherwise it is higher than 0x0. And save NMI
super prioirty information in hppi.superprio to deliver NMI exception.
Since both GICR and GICD can deliver NMI, it is both necessary to check
whether the pending irq is NMI in gicv3_redist_update_noirqset and
gicv3_update_noirqset. And In irqbetter(), only a non-NMI with the same
priority and a smaller interrupt number can be preempted but not NMI.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v8:
- Add Reviewed-by.
v7:
- Reorder the irqbetter() code for clarity.
- Eliminate the has_superprio local variable for gicv3_get_priority().
- false -> cs->hpplpi.superprio in gicv3_redist_update_noirqset().
- 0x0 -> false in arm_gicv3_common_reset_hold().
- Clear superprio in several places for hppi, hpplpi and hppvlpi.
v6:
- Put the "extract superprio info" logic into gicv3_get_priority().
- Update the comment in irqbetter().
- Reset the cs->hppi.superprio to 0x0.
- Set hppi.superprio to false for LPI.
v4:
- Replace is_nmi with has_superprio to not a mix NMI and superpriority.
- Update the comment in irqbetter().
- Extract gicv3_get_priority() to avoid code repeat.
---
v3:
- Add missing brace
---
 hw/intc/arm_gicv3.c| 69 +-
 hw/intc/arm_gicv3_common.c |  3 ++
 hw/intc/arm_gicv3_redist.c |  3 ++
 3 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
index 0b8f79a122..9496a28005 100644
--- a/hw/intc/arm_gicv3.c
+++ b/hw/intc/arm_gicv3.c
@@ -21,7 +21,8 @@
 #include "hw/intc/arm_gicv3.h"
 #include "gicv3_internal.h"
 
-static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
+static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio,
+  bool has_superprio)
 {
 /* Return true if this IRQ at this priority should take
  * precedence over the current recorded highest priority
@@ -30,14 +31,23 @@ static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t 
prio)
  * is the same as this one (a property which the calling code
  * relies on).
  */
-if (prio < cs->hppi.prio) {
-return true;
+if (prio != cs->hppi.prio) {
+return prio < cs->hppi.prio;
+}
+
+/*
+ * The same priority IRQ with superpriority should signal to the CPU
+ * as it have the priority higher than the labelled 0x80 or 0x00.
+ */
+if (has_superprio != cs->hppi.superprio) {
+return has_superprio;
 }
+
 /* If multiple pending interrupts have the same priority then it is an
  * IMPDEF choice which of them to signal to the CPU. We choose to
  * signal the one with the lowest interrupt number.
  */
-if (prio == cs->hppi.prio && irq <= cs->hppi.irq) {
+if (irq <= cs->hppi.irq) {
 return true;
 }
 return false;
@@ -129,6 +139,40 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs)
 return pend;
 }
 
+static bool gicv3_get_priority(GICv3CPUState *cs, bool is_redist,
+   uint8_t *prio, int irq)
+{
+uint32_t superprio = 0x0;
+
+if (is_redist) {
+superprio = extract32(cs->gicr_isuperprio, irq, 1);
+} else {
+superprio = *gic_bmp_ptr32(cs->gic->superprio, irq);
+superprio = superprio & (1 << (irq & 0x1f));
+}
+
+if (superprio) {
+/* DS = 0 & Non-secure NMI */
+if (!(cs->gic->gicd_ctlr & GICD_CTLR_DS) &&
+((is_redist && extract32(cs->gicr_igroupr0, irq, 1)) ||
+ (!is_redist && gicv3_gicd_group_test(cs->gic, irq {
+*prio = 0x80;
+} else {
+*prio = 0x0;
+}
+
+return true;
+}
+
+if (is_redist) {
+*prio = cs->gicr_ipriorityr[irq];
+} else {
+*prio = cs->gic->gicd_ipriority[irq];
+}
+
+return false;
+}
+
 /* Update the interrupt status after state in a redistributor
  * or CPU interface has changed, but don't tell the CPU i/f.
  */
@@ -141,6 +185,7 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
 uint8_t prio;
 int i;
 uint32_t pend;
+bool has_superprio = false;
 
 /* Find out which redistributor interrupts are eligible to be
  * signaled to the CPU interface.
@@ -152,10 +197,11 @@ static void gicv3_redist_update_noirqset(GICv3CPUState 
*cs)
 if (!(pend & (1 << i))) {
 continue;
 }
-prio = cs->gicr_ipriorityr[i];
-if (irqbetter(cs, i, prio)) {
+has_superprio = gicv3_get_priority(cs, true, , i);
+if (irqbetter(cs, i, prio, has_superprio)) {
 cs->hppi.irq = i;
 cs->hppi.prio = prio;
+cs->hppi.superprio = has_superprio;
 seenbetter = true;
 }
 }
@@ -168,9 +214,11 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
 if ((cs->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) 

[RFC PATCH v8 20/23] hw/intc/arm_gicv3: Report the NMI interrupt in gicv3_cpuif_update()

[Jinjie Ruan via]

In CPU Interface, if the IRQ has the superpriority property, report
NMI to the corresponding PE.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v6:
- Add Reviewed-by.
v4:
- Swap the ordering of the IFs.
v3:
- Remove handling nmi_is_irq flag.
---
 hw/intc/arm_gicv3_cpuif.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index d67d62359b..29142fcf8d 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -969,6 +969,7 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
 /* Tell the CPU about its highest priority pending interrupt */
 int irqlevel = 0;
 int fiqlevel = 0;
+int nmilevel = 0;
 ARMCPU *cpu = ARM_CPU(cs->cpu);
 CPUARMState *env = >env;
 
@@ -1007,6 +1008,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
 
 if (isfiq) {
 fiqlevel = 1;
+} else if (cs->hppi.superprio) {
+nmilevel = 1;
 } else {
 irqlevel = 1;
 }
@@ -1016,6 +1019,7 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
 
 qemu_set_irq(cs->parent_fiq, fiqlevel);
 qemu_set_irq(cs->parent_irq, irqlevel);
+qemu_set_irq(cs->parent_nmi, nmilevel);
 }
 
 static uint64_t icc_pmr_read(CPUARMState *env, const ARMCPRegInfo *ri)
-- 
2.34.1

[RFC PATCH v8 09/23] target/arm: Handle PSTATE.ALLINT on taking an exception

[Jinjie Ruan via]

Set or clear PSTATE.ALLINT on taking an exception to ELx according to the
SCTLR_ELx.SPINTMASK bit.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v3:
- Add Reviewed-by.
---
 target/arm/helper.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 4bc63bf7ca..81f4a8f194 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11705,6 +11705,15 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
 }
 }
 
+if (cpu_isar_feature(aa64_nmi, cpu) &&
+(env->cp15.sctlr_el[new_el] & SCTLR_NMI)) {
+if (!(env->cp15.sctlr_el[new_el] & SCTLR_SPINTMASK)) {
+new_mode |= PSTATE_ALLINT;
+} else {
+new_mode &= ~PSTATE_ALLINT;
+}
+}
+
 pstate_write(env, PSTATE_DAIF | new_mode);
 env->aarch64 = true;
 aarch64_restore_sp(env, new_el);
-- 
2.34.1

[RFC PATCH v8 05/23] target/arm: Support MSR access to ALLINT

[Jinjie Ruan via]

Support ALLINT msr access as follow:
mrs , ALLINT// read allint
msr ALLINT, // write allint with imm

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v5:
- Add Reviewed-by.
v4:
- Remove arm_is_el2_enabled() check in allint_check().
- Change to env->pstate instead of env->allint.
v3:
- Remove EL0 check in aa64_allint_access() which alreay checks in .access
  PL1_RW.
- Use arm_hcrx_el2_eff() in aa64_allint_access() instead of env->cp15.hcrx_el2.
- Make ALLINT msr access function controlled by aa64_nmi.
---
 target/arm/helper.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b19a0178ce..aa0151c775 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4752,6 +4752,36 @@ static void aa64_daif_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 env->daif = value & PSTATE_DAIF;
 }
 
+static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo *ri,
+  uint64_t value)
+{
+env->pstate = (env->pstate & ~PSTATE_ALLINT) | (value & PSTATE_ALLINT);
+}
+
+static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+return env->pstate & PSTATE_ALLINT;
+}
+
+static CPAccessResult aa64_allint_access(CPUARMState *env,
+ const ARMCPRegInfo *ri, bool isread)
+{
+if (arm_current_el(env) == 1 && (arm_hcrx_el2_eff(env) & HCRX_TALLINT)) {
+return CP_ACCESS_TRAP_EL2;
+}
+return CP_ACCESS_OK;
+}
+
+static const ARMCPRegInfo nmi_reginfo[] = {
+{ .name = "ALLINT", .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 4, .crm = 3,
+  .type = ARM_CP_NO_RAW,
+  .access = PL1_RW, .accessfn = aa64_allint_access,
+  .fieldoffset = offsetof(CPUARMState, pstate),
+  .writefn = aa64_allint_write, .readfn = aa64_allint_read,
+  .resetfn = arm_cp_reset_ignore },
+};
+
 static uint64_t aa64_pan_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
 return env->pstate & PSTATE_PAN;
@@ -9889,6 +9919,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 if (cpu_isar_feature(aa64_nv2, cpu)) {
 define_arm_cp_regs(cpu, nv2_reginfo);
 }
+
+if (cpu_isar_feature(aa64_nmi, cpu)) {
+define_arm_cp_regs(cpu, nmi_reginfo);
+}
 #endif
 
 if (cpu_isar_feature(any_predinv, cpu)) {
-- 
2.34.1

[RFC PATCH v8 14/23] hw/intc/arm_gicv3_redist: Implement GICR_INMIR0

[Jinjie Ruan via]

Add GICR_INMIR0 register and support access GICR_INMIR0.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v6:
- Add Reviewed-by.
v4:
- Make the GICR_INMIR0 implementation more clearer.
---
 hw/intc/arm_gicv3_redist.c | 19 +++
 hw/intc/gicv3_internal.h   |  1 +
 2 files changed, 20 insertions(+)

diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index 8153525849..7a16a058b1 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -35,6 +35,15 @@ static int gicr_ns_access(GICv3CPUState *cs, int irq)
 return extract32(cs->gicr_nsacr, irq * 2, 2);
 }
 
+static void gicr_write_bitmap_reg(GICv3CPUState *cs, MemTxAttrs attrs,
+  uint32_t *reg, uint32_t val)
+{
+/* Helper routine to implement writing to a "set" register */
+val &= mask_group(cs, attrs);
+*reg = val;
+gicv3_redist_update(cs);
+}
+
 static void gicr_write_set_bitmap_reg(GICv3CPUState *cs, MemTxAttrs attrs,
   uint32_t *reg, uint32_t val)
 {
@@ -406,6 +415,10 @@ static MemTxResult gicr_readl(GICv3CPUState *cs, hwaddr 
offset,
 *data = value;
 return MEMTX_OK;
 }
+case GICR_INMIR0:
+*data = cs->gic->nmi_support ?
+gicr_read_bitmap_reg(cs, attrs, cs->gicr_isuperprio) : 0;
+return MEMTX_OK;
 case GICR_ICFGR0:
 case GICR_ICFGR1:
 {
@@ -555,6 +568,12 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr 
offset,
 gicv3_redist_update(cs);
 return MEMTX_OK;
 }
+case GICR_INMIR0:
+if (cs->gic->nmi_support) {
+gicr_write_bitmap_reg(cs, attrs, >gicr_isuperprio, value);
+}
+return MEMTX_OK;
+
 case GICR_ICFGR0:
 /* Register is all RAZ/WI or RAO/WI bits */
 return MEMTX_OK;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 29d5cdc1b6..f35b7d2f03 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -109,6 +109,7 @@
 #define GICR_ICFGR1   (GICR_SGI_OFFSET + 0x0C04)
 #define GICR_IGRPMODR0(GICR_SGI_OFFSET + 0x0D00)
 #define GICR_NSACR(GICR_SGI_OFFSET + 0x0E00)
+#define GICR_INMIR0   (GICR_SGI_OFFSET + 0x0F80)
 
 /* VLPI redistributor registers, offsets from VLPI_base */
 #define GICR_VPROPBASER   (GICR_VLPI_OFFSET + 0x70)
-- 
2.34.1

[RFC PATCH v8 16/23] hw/intc: Enable FEAT_GICv3_NMI Feature

[Jinjie Ruan via]

Added properties to enable FEAT_GICv3_NMI feature, setup distributor
and redistributor registers to indicate NMI support.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v4:
- Add Reviewed-by.
---
 hw/intc/arm_gicv3_common.c | 1 +
 hw/intc/arm_gicv3_dist.c   | 2 ++
 hw/intc/gicv3_internal.h   | 1 +
 include/hw/intc/arm_gicv3_common.h | 1 +
 4 files changed, 5 insertions(+)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index c52f060026..2d2cea6858 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -569,6 +569,7 @@ static Property arm_gicv3_common_properties[] = {
 DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
 DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
 DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
+DEFINE_PROP_BOOL("has-nmi", GICv3State, nmi_support, 0),
 DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
 /*
  * Compatibility property: force 8 bits of physical priority, even
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index 9739404e35..c4e28d209a 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -412,6 +412,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
  *  by GICD_TYPER.IDbits)
  * MBIS == 0 (message-based SPIs not supported)
  * SecurityExtn == 1 if security extns supported
+ * NMI = 1 if Non-maskable interrupt property is supported
  * CPUNumber == 0 since for us ARE is always 1
  * ITLinesNumber == (((max SPI IntID + 1) / 32) - 1)
  */
@@ -425,6 +426,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
 bool dvis = s->revision >= 4;
 
 *data = (1 << 25) | (1 << 24) | (dvis << 18) | (sec_extn << 10) |
+(s->nmi_support << GICD_TYPER_NMI_SHIFT) |
 (s->lpi_enable << GICD_TYPER_LPIS_SHIFT) |
 (0xf << 19) | itlinesnumber;
 return true;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index a1fc34597e..8d793243f4 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -70,6 +70,7 @@
 #define GICD_CTLR_E1NWF (1U << 7)
 #define GICD_CTLR_RWP   (1U << 31)
 
+#define GICD_TYPER_NMI_SHIFT   9
 #define GICD_TYPER_LPIS_SHIFT  17
 
 /* 16 bits EventId */
diff --git a/include/hw/intc/arm_gicv3_common.h 
b/include/hw/intc/arm_gicv3_common.h
index df4380141d..16c5fa7256 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -251,6 +251,7 @@ struct GICv3State {
 uint32_t num_irq;
 uint32_t revision;
 bool lpi_enable;
+bool nmi_support;
 bool security_extn;
 bool force_8bit_prio;
 bool irq_reset_nonsecure;
-- 
2.34.1

[RFC PATCH v8 12/23] target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()

[Jinjie Ruan via]

According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
with superpriority is always IRQ, never FIQ, so the NMI exception trap entry
behave like IRQ. And VNMI(vIRQ with Superpriority) can be raised from the
GIC or come from the hcrx_el2.HCRX_VINMI bit.

Signed-off-by: Jinjie Ruan 
Reviewed-by: Richard Henderson 
---
v7:
- Add Reviewed-by.
v6:
- Not combine VFNMI with CPU_INTERRUPT_VNMI.
v4:
- Also handle VNMI in arm_cpu_do_interrupt_aarch64().
v3:
- Remove the FIQ NMI handle.
---
 target/arm/helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 81f4a8f194..e279b26e9d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11625,6 +11625,8 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
 break;
 case EXCP_IRQ:
 case EXCP_VIRQ:
+case EXCP_NMI:
+case EXCP_VNMI:
 addr += 0x80;
 break;
 case EXCP_FIQ:
-- 
2.34.1

Re: [PATCH-for-9.1 0/3] ui/display: Introduce API to change console orientation

[Philippe Mathieu-Daudé]

(Forgot to Cc Akihiko)

On 18/3/24 11:05, Philippe Mathieu-Daudé wrote:

Hi,

The idea behind this series is to reduce the use of the
'graphic_rotate' global. It is only used by the Spitz
machine, so we could convert the '-rotate' argument to
a sugar property on the PXA2XX_LCD_TYPE model, but since
the Spitz machine has been deprecated recently (commit
a2531bb855 "Deprecate various old Arm machine types") it
doesn't seem worthwhile. So just extract the API to change
console orientation.

Regards,

Phil.

Philippe Mathieu-Daudé (3):
   ui/console: Introduce API to change console orientation
   hw/display/pxa2xx_lcd: Set rotation angle using
 qemu_console_set_rotate
   ui/console: Add 'rotate_arcdegree' field to allow per-console rotation

[PATCH-for-9.1 v2 1/3] ui/console: Introduce API to change console orientation

[Philippe Mathieu-Daudé]

Extract the following methods:

  - qemu_console_set_rotate()
  - qemu_console_is_rotated()
  - qemu_console_get_rotate_arcdegree()

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/ui/console.h |  3 +++
 ui/console.c | 16 
 ui/input.c   |  9 -
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index a4a49ffc64..86ba36e391 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -422,15 +422,18 @@ bool qemu_console_is_visible(QemuConsole *con);
 bool qemu_console_is_graphic(QemuConsole *con);
 bool qemu_console_is_fixedsize(QemuConsole *con);
 bool qemu_console_is_gl_blocked(QemuConsole *con);
+bool qemu_console_is_rotated(QemuConsole *con);
 char *qemu_console_get_label(QemuConsole *con);
 int qemu_console_get_index(QemuConsole *con);
 uint32_t qemu_console_get_head(QemuConsole *con);
 int qemu_console_get_width(QemuConsole *con, int fallback);
 int qemu_console_get_height(QemuConsole *con, int fallback);
+unsigned qemu_console_get_rotate_arcdegree(QemuConsole *con);
 /* Return the low-level window id for the console */
 int qemu_console_get_window_id(QemuConsole *con);
 /* Set the low-level window id for the console */
 void qemu_console_set_window_id(QemuConsole *con, int window_id);
+void qemu_console_set_rotate(QemuConsole *con, unsigned arcdegree);
 
 void console_select(unsigned int index);
 void qemu_console_resize(QemuConsole *con, int width, int height);
diff --git a/ui/console.c b/ui/console.c
index 832055675c..84aee76846 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -37,6 +37,7 @@
 #include "trace.h"
 #include "exec/memory.h"
 #include "qom/object.h"
+#include "sysemu/sysemu.h"
 
 #include "console-priv.h"
 
@@ -207,6 +208,21 @@ void qemu_console_set_window_id(QemuConsole *con, int 
window_id)
 con->window_id = window_id;
 }
 
+void qemu_console_set_rotate(QemuConsole *con, unsigned arcdegree)
+{
+graphic_rotate = arcdegree;
+}
+
+bool qemu_console_is_rotated(QemuConsole *con)
+{
+return graphic_rotate != 0;
+}
+
+unsigned qemu_console_get_rotate_arcdegree(QemuConsole *con)
+{
+return graphic_rotate;
+}
+
 void graphic_hw_invalidate(QemuConsole *con)
 {
 if (!con) {
diff --git a/ui/input.c b/ui/input.c
index dc745860f4..951806bf05 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -1,5 +1,4 @@
 #include "qemu/osdep.h"
-#include "sysemu/sysemu.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-ui.h"
 #include "trace.h"
@@ -179,10 +178,10 @@ static int qemu_input_transform_invert_abs_value(int 
value)
   return (int64_t)INPUT_EVENT_ABS_MAX - value + INPUT_EVENT_ABS_MIN;
 }
 
-static void qemu_input_transform_abs_rotate(InputEvent *evt)
+static void qemu_input_transform_abs_rotate(QemuConsole *src, InputEvent *evt)
 {
 InputMoveEvent *move = evt->u.abs.data;
-switch (graphic_rotate) {
+switch (qemu_console_get_rotate_arcdegree(src)) {
 case 90:
 if (move->axis == INPUT_AXIS_X) {
 move->axis = INPUT_AXIS_Y;
@@ -341,8 +340,8 @@ void qemu_input_event_send_impl(QemuConsole *src, 
InputEvent *evt)
 qemu_input_event_trace(src, evt);
 
 /* pre processing */
-if (graphic_rotate && (evt->type == INPUT_EVENT_KIND_ABS)) {
-qemu_input_transform_abs_rotate(evt);
+if (qemu_console_is_rotated(src) && (evt->type == INPUT_EVENT_KIND_ABS)) {
+qemu_input_transform_abs_rotate(src, evt);
 }
 
 /* send event */
-- 
2.41.0

[PATCH-for-9.1 v2 3/3] ui/console: Add 'rotate_arcdegree' field to allow per-console rotation

[Philippe Mathieu-Daudé]

Add the 'rotate_arcdegree' field to QemuGraphicConsole and
remove the use of the 'graphic_rotate' global.

Signed-off-by: Philippe Mathieu-Daudé 
---
 ui/console.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index 84aee76846..94624c37ae 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -37,7 +37,6 @@
 #include "trace.h"
 #include "exec/memory.h"
 #include "qom/object.h"
-#include "sysemu/sysemu.h"
 
 #include "console-priv.h"
 
@@ -51,6 +50,8 @@ typedef struct QemuGraphicConsole {
 
 QEMUCursor *cursor;
 int cursor_x, cursor_y, cursor_on;
+
+unsigned rotate_arcdegree;
 } QemuGraphicConsole;
 
 typedef QemuConsoleClass QemuGraphicConsoleClass;
@@ -210,17 +211,23 @@ void qemu_console_set_window_id(QemuConsole *con, int 
window_id)
 
 void qemu_console_set_rotate(QemuConsole *con, unsigned arcdegree)
 {
-graphic_rotate = arcdegree;
+QemuGraphicConsole *gc = QEMU_GRAPHIC_CONSOLE(con);
+
+gc->rotate_arcdegree = arcdegree;
 }
 
 bool qemu_console_is_rotated(QemuConsole *con)
 {
-return graphic_rotate != 0;
+QemuGraphicConsole *gc = QEMU_GRAPHIC_CONSOLE(con);
+
+return gc->rotate_arcdegree != 0;
 }
 
 unsigned qemu_console_get_rotate_arcdegree(QemuConsole *con)
 {
-return graphic_rotate;
+QemuGraphicConsole *gc = QEMU_GRAPHIC_CONSOLE(con);
+
+return gc->rotate_arcdegree;
 }
 
 void graphic_hw_invalidate(QemuConsole *con)
-- 
2.41.0