From e561b3b7df11875d7bd3b8670598cae5601a7f70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 5 Jun 2022 10:51:30 +0200 Subject: [PATCH 01/15] ui/gtk-gl-area: implement GL context destruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The counterpart function for gd_gl_area_create_context() is currently empty. Implement the gd_gl_area_destroy_context() function to avoid GL context leaks. Signed-off-by: Volker Rümelin Message-Id: <20220605085131.7711-1-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- ui/gtk-gl-area.c | 8 +++++++- ui/trace-events | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index fc5a082eb8..0e20ea031d 100644 --- a/ui/gtk-gl-area.c +++ b/ui/gtk-gl-area.c @@ -201,7 +201,13 @@ QEMUGLContext gd_gl_area_create_context(DisplayGLCtx *dgc, void gd_gl_area_destroy_context(DisplayGLCtx *dgc, QEMUGLContext ctx) { - /* FIXME */ + GdkGLContext *current_ctx = gdk_gl_context_get_current(); + + trace_gd_gl_area_destroy_context(ctx, current_ctx); + if (ctx == current_ctx) { + gdk_gl_context_clear_current(); + } + g_clear_object(&ctx); } void gd_gl_area_scanout_texture(DisplayChangeListener *dcl, diff --git a/ui/trace-events b/ui/trace-events index f78b5e6606..1040ba0f88 100644 --- a/ui/trace-events +++ b/ui/trace-events @@ -26,6 +26,7 @@ gd_key_event(const char *tab, int gdk_keycode, int qkeycode, const char *action) gd_grab(const char *tab, const char *device, const char *reason) "tab=%s, dev=%s, reason=%s" gd_ungrab(const char *tab, const char *device) "tab=%s, dev=%s" gd_keymap_windowing(const char *name) "backend=%s" +gd_gl_area_destroy_context(void *ctx, void *current_ctx) "ctx=%p, current_ctx=%p" # vnc-auth-sasl.c # vnc-auth-vencrypt.c From 09053670c9b83f1f4759f37e62a6f2b1a5bbd105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 5 Jun 2022 10:51:31 +0200 Subject: [PATCH 02/15] ui/gtk-gl-area: create the requested GL context version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since about 2018 virglrenderer (commit fa835b0f88 "vrend: don't hardcode context version") tries to open the highest available GL context version. This is done by creating the known GL context versions from the highest to the lowest until (*create_gl_context) returns a context != NULL. This does not work properly with the current QEMU gd_gl_area_create_context() function, because gdk_gl_context_realize() on Wayland creates a version 3.0 legacy context if the requested GL context version can't be created. In order for virglrenderer to find the highest available GL context version, return NULL if the created context version is lower than the requested version. This fixes the following error: QEMU started with -device virtio-vga-gl -display gtk,gl=on. Under Wayland, the guest window remains black and the following information can be seen on the host. gl_version 30 - compat profile (qemu:5978): Gdk-WARNING **: 16:19:01.533: gdk_gl_context_set_required_version - GL context versions less than 3.2 are not supported. (qemu:5978): Gdk-WARNING **: 16:19:01.537: gdk_gl_context_set_required_version - GL context versions less than 3.2 are not supported. (qemu:5978): Gdk-WARNING **: 16:19:01.554: gdk_gl_context_set_required_version - GL context versions less than 3.2 are not supported. vrend_renderer_fill_caps: Entering with stale GL error: 1282 To reproduce this error, an OpenGL driver is required on the host that doesn't have the latest OpenGL extensions fully implemented. An example for this is the Intel i965 driver on a Haswell processor. Signed-off-by: Volker Rümelin Message-Id: <20220605085131.7711-2-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- ui/gtk-gl-area.c | 31 ++++++++++++++++++++++++++++++- ui/trace-events | 1 + 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index 0e20ea031d..2e0129c28c 100644 --- a/ui/gtk-gl-area.c +++ b/ui/gtk-gl-area.c @@ -170,6 +170,23 @@ void gd_gl_area_switch(DisplayChangeListener *dcl, } } +static int gd_cmp_gl_context_version(int major, int minor, QEMUGLParams *params) +{ + if (major > params->major_ver) { + return 1; + } + if (major < params->major_ver) { + return -1; + } + if (minor > params->minor_ver) { + return 1; + } + if (minor < params->minor_ver) { + return -1; + } + return 0; +} + QEMUGLContext gd_gl_area_create_context(DisplayGLCtx *dgc, QEMUGLParams *params) { @@ -177,8 +194,8 @@ QEMUGLContext gd_gl_area_create_context(DisplayGLCtx *dgc, GdkWindow *window; GdkGLContext *ctx; GError *err = NULL; + int major, minor; - gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area)); window = gtk_widget_get_window(vc->gfx.drawing_area); ctx = gdk_window_create_gl_context(window, &err); if (err) { @@ -196,6 +213,18 @@ QEMUGLContext gd_gl_area_create_context(DisplayGLCtx *dgc, g_clear_object(&ctx); return NULL; } + + gdk_gl_context_make_current(ctx); + gdk_gl_context_get_version(ctx, &major, &minor); + gdk_gl_context_clear_current(); + gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area)); + + if (gd_cmp_gl_context_version(major, minor, params) == -1) { + /* created ctx version < requested version */ + g_clear_object(&ctx); + } + + trace_gd_gl_area_create_context(ctx, params->major_ver, params->minor_ver); return ctx; } diff --git a/ui/trace-events b/ui/trace-events index 1040ba0f88..a922f00e10 100644 --- a/ui/trace-events +++ b/ui/trace-events @@ -26,6 +26,7 @@ gd_key_event(const char *tab, int gdk_keycode, int qkeycode, const char *action) gd_grab(const char *tab, const char *device, const char *reason) "tab=%s, dev=%s, reason=%s" gd_ungrab(const char *tab, const char *device) "tab=%s, dev=%s" gd_keymap_windowing(const char *name) "backend=%s" +gd_gl_area_create_context(void *ctx, int major, int minor) "ctx=%p, major=%d, minor=%d" gd_gl_area_destroy_context(void *ctx, void *current_ctx) "ctx=%p, current_ctx=%p" # vnc-auth-sasl.c From 2910abd6b4364ee96d2339b2be8858246fcd3161 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 29 May 2022 17:25:08 +0900 Subject: [PATCH 03/15] ui/cocoa: Fix poweroff request code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Akihiko Odaki Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20220529082508.89097-1-akihiko.odaki@gmail.com> Signed-off-by: Gerd Hoffmann --- ui/cocoa.m | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 09a62817f2..84c84e98fc 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -35,6 +35,7 @@ #include "ui/kbd-state.h" #include "sysemu/sysemu.h" #include "sysemu/runstate.h" +#include "sysemu/runstate-action.h" #include "sysemu/cpu-throttle.h" #include "qapi/error.h" #include "qapi/qapi-commands-block.h" @@ -1290,7 +1291,10 @@ static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEven { COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n"); - qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI); + with_iothread_lock(^{ + shutdown_action = SHUTDOWN_ACTION_POWEROFF; + qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI); + }); /* * Sleep here, because returning will cause OSX to kill us From d7d34918551dc48a2f6b42b44a12c7a9183f89e6 Mon Sep 17 00:00:00 2001 From: "Hongren (Zenithal) Zheng" Date: Thu, 19 May 2022 20:38:10 +0800 Subject: [PATCH 04/15] hw/usb: Add CanoKey Implementation This commit added a new emulated device called CanoKey to QEMU. CanoKey implements platform independent features in canokey-core https://github.com/canokeys/canokey-core, and leaves the USB implementation to the platform. In this commit the USB part was implemented in QEMU using QEMU's USB APIs, therefore the emulated CanoKey can communicate with the guest OS using USB. Signed-off-by: Hongren (Zenithal) Zheng Message-Id: Signed-off-by: Gerd Hoffmann --- hw/usb/canokey.c | 300 +++++++++++++++++++++++++++++++++++++++++++++++ hw/usb/canokey.h | 69 +++++++++++ 2 files changed, 369 insertions(+) create mode 100644 hw/usb/canokey.c create mode 100644 hw/usb/canokey.h diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c new file mode 100644 index 0000000000..6cb8b7cdb0 --- /dev/null +++ b/hw/usb/canokey.c @@ -0,0 +1,300 @@ +/* + * CanoKey QEMU device implementation. + * + * Copyright (c) 2021-2022 Canokeys.org + * Written by Hongren (Zenithal) Zheng + * + * This code is licensed under the Apache-2.0. + */ + +#include "qemu/osdep.h" +#include + +#include "qemu/module.h" +#include "qapi/error.h" +#include "hw/usb.h" +#include "hw/qdev-properties.h" +#include "desc.h" +#include "canokey.h" + +#define CANOKEY_EP_IN(ep) ((ep) & 0x7F) + +#define CANOKEY_VENDOR_NUM 0x20a0 +#define CANOKEY_PRODUCT_NUM 0x42d2 + +/* + * placeholder, canokey-qemu implements its own usb desc + * Namely we do not use usb_desc_handle_contorl + */ +enum { + STR_MANUFACTURER = 1, + STR_PRODUCT, + STR_SERIALNUMBER +}; + +static const USBDescStrings desc_strings = { + [STR_MANUFACTURER] = "canokeys.org", + [STR_PRODUCT] = "CanoKey QEMU", + [STR_SERIALNUMBER] = "0" +}; + +static const USBDescDevice desc_device_canokey = { + .bcdUSB = 0x0, + .bMaxPacketSize0 = 16, + .bNumConfigurations = 0, + .confs = NULL, +}; + +static const USBDesc desc_canokey = { + .id = { + .idVendor = CANOKEY_VENDOR_NUM, + .idProduct = CANOKEY_PRODUCT_NUM, + .bcdDevice = 0x0100, + .iManufacturer = STR_MANUFACTURER, + .iProduct = STR_PRODUCT, + .iSerialNumber = STR_SERIALNUMBER, + }, + .full = &desc_device_canokey, + .high = &desc_device_canokey, + .str = desc_strings, +}; + + +/* + * libcanokey-qemu.so side functions + * All functions are called from canokey_emu_device_loop + */ +int canokey_emu_stall_ep(void *base, uint8_t ep) +{ + CanoKeyState *key = base; + uint8_t ep_in = CANOKEY_EP_IN(ep); /* INTR IN has ep 129 */ + key->ep_in_size[ep_in] = 0; + key->ep_in_state[ep_in] = CANOKEY_EP_IN_STALL; + return 0; +} + +int canokey_emu_set_address(void *base, uint8_t addr) +{ + CanoKeyState *key = base; + key->dev.addr = addr; + return 0; +} + +int canokey_emu_prepare_receive( + void *base, uint8_t ep, uint8_t *pbuf, uint16_t size) +{ + CanoKeyState *key = base; + key->ep_out[ep] = pbuf; + key->ep_out_size[ep] = size; + return 0; +} + +int canokey_emu_transmit( + void *base, uint8_t ep, const uint8_t *pbuf, uint16_t size) +{ + CanoKeyState *key = base; + uint8_t ep_in = CANOKEY_EP_IN(ep); /* INTR IN has ep 129 */ + memcpy(key->ep_in[ep_in] + key->ep_in_size[ep_in], + pbuf, size); + key->ep_in_size[ep_in] += size; + key->ep_in_state[ep_in] = CANOKEY_EP_IN_READY; + /* + * ready for more data in device loop + * + * Note: this is a quirk for CanoKey CTAPHID + * because it calls multiple emu_transmit in one device_loop + * but w/o data_in it would stuck in device_loop + * This has no side effect for CCID as it is strictly + * OUT then IN transfer + * However it has side effect for Control transfer + */ + if (ep_in != 0) { + canokey_emu_data_in(ep_in); + } + return 0; +} + +uint32_t canokey_emu_get_rx_data_size(void *base, uint8_t ep) +{ + CanoKeyState *key = base; + return key->ep_out_size[ep]; +} + +/* + * QEMU side functions + */ +static void canokey_handle_reset(USBDevice *dev) +{ + CanoKeyState *key = CANOKEY(dev); + for (int i = 0; i != CANOKEY_EP_NUM; ++i) { + key->ep_in_state[i] = CANOKEY_EP_IN_WAIT; + key->ep_in_pos[i] = 0; + key->ep_in_size[i] = 0; + } + canokey_emu_reset(); +} + +static void canokey_handle_control(USBDevice *dev, USBPacket *p, + int request, int value, int index, int length, uint8_t *data) +{ + CanoKeyState *key = CANOKEY(dev); + + canokey_emu_setup(request, value, index, length); + + uint32_t dir_in = request & DeviceRequest; + if (!dir_in) { + /* OUT */ + if (key->ep_out[0] != NULL) { + memcpy(key->ep_out[0], data, length); + } + canokey_emu_data_out(p->ep->nr, data); + } + + canokey_emu_device_loop(); + + /* IN */ + switch (key->ep_in_state[0]) { + case CANOKEY_EP_IN_WAIT: + p->status = USB_RET_NAK; + break; + case CANOKEY_EP_IN_STALL: + p->status = USB_RET_STALL; + break; + case CANOKEY_EP_IN_READY: + memcpy(data, key->ep_in[0], key->ep_in_size[0]); + p->actual_length = key->ep_in_size[0]; + /* reset state */ + key->ep_in_state[0] = CANOKEY_EP_IN_WAIT; + key->ep_in_size[0] = 0; + key->ep_in_pos[0] = 0; + break; + } +} + +static void canokey_handle_data(USBDevice *dev, USBPacket *p) +{ + CanoKeyState *key = CANOKEY(dev); + + uint8_t ep_in = CANOKEY_EP_IN(p->ep->nr); + uint8_t ep_out = p->ep->nr; + uint32_t in_len; + uint32_t out_pos; + uint32_t out_len; + switch (p->pid) { + case USB_TOKEN_OUT: + usb_packet_copy(p, key->ep_out_buffer[ep_out], p->iov.size); + out_pos = 0; + while (out_pos != p->iov.size) { + /* + * key->ep_out[ep_out] set by prepare_receive + * to be a buffer inside libcanokey-qemu.so + * key->ep_out_size[ep_out] set by prepare_receive + * to be the buffer length + */ + out_len = MIN(p->iov.size - out_pos, key->ep_out_size[ep_out]); + memcpy(key->ep_out[ep_out], + key->ep_out_buffer[ep_out] + out_pos, out_len); + out_pos += out_len; + /* update ep_out_size to actual len */ + key->ep_out_size[ep_out] = out_len; + canokey_emu_data_out(ep_out, NULL); + } + break; + case USB_TOKEN_IN: + if (key->ep_in_pos[ep_in] == 0) { /* first time IN */ + canokey_emu_data_in(ep_in); + canokey_emu_device_loop(); /* may call transmit multiple times */ + } + switch (key->ep_in_state[ep_in]) { + case CANOKEY_EP_IN_WAIT: + /* NAK for early INTR IN */ + p->status = USB_RET_NAK; + break; + case CANOKEY_EP_IN_STALL: + p->status = USB_RET_STALL; + break; + case CANOKEY_EP_IN_READY: + /* submit part of ep_in buffer to USBPacket */ + in_len = MIN(key->ep_in_size[ep_in] - key->ep_in_pos[ep_in], + p->iov.size); + usb_packet_copy(p, + key->ep_in[ep_in] + key->ep_in_pos[ep_in], in_len); + key->ep_in_pos[ep_in] += in_len; + /* reset state if all data submitted */ + if (key->ep_in_pos[ep_in] == key->ep_in_size[ep_in]) { + key->ep_in_state[ep_in] = CANOKEY_EP_IN_WAIT; + key->ep_in_size[ep_in] = 0; + key->ep_in_pos[ep_in] = 0; + } + break; + } + break; + default: + p->status = USB_RET_STALL; + break; + } +} + +static void canokey_realize(USBDevice *base, Error **errp) +{ + CanoKeyState *key = CANOKEY(base); + + if (key->file == NULL) { + error_setg(errp, "You must provide file=/path/to/canokey-file"); + return; + } + + usb_desc_init(base); + + for (int i = 0; i != CANOKEY_EP_NUM; ++i) { + key->ep_in_state[i] = CANOKEY_EP_IN_WAIT; + key->ep_in_size[i] = 0; + key->ep_in_pos[i] = 0; + } + + if (canokey_emu_init(key, key->file)) { + error_setg(errp, "canokey can not create or read %s", key->file); + return; + } +} + +static void canokey_unrealize(USBDevice *base) +{ +} + +static Property canokey_properties[] = { + DEFINE_PROP_STRING("file", CanoKeyState, file), + DEFINE_PROP_END_OF_LIST(), +}; + +static void canokey_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + USBDeviceClass *uc = USB_DEVICE_CLASS(klass); + + uc->product_desc = "CanoKey QEMU"; + uc->usb_desc = &desc_canokey; + uc->handle_reset = canokey_handle_reset; + uc->handle_control = canokey_handle_control; + uc->handle_data = canokey_handle_data; + uc->handle_attach = usb_desc_attach; + uc->realize = canokey_realize; + uc->unrealize = canokey_unrealize; + dc->desc = "CanoKey QEMU"; + device_class_set_props(dc, canokey_properties); + set_bit(DEVICE_CATEGORY_MISC, dc->categories); +} + +static const TypeInfo canokey_info = { + .name = TYPE_CANOKEY, + .parent = TYPE_USB_DEVICE, + .instance_size = sizeof(CanoKeyState), + .class_init = canokey_class_init +}; + +static void canokey_register_types(void) +{ + type_register_static(&canokey_info); +} + +type_init(canokey_register_types) diff --git a/hw/usb/canokey.h b/hw/usb/canokey.h new file mode 100644 index 0000000000..24cf304203 --- /dev/null +++ b/hw/usb/canokey.h @@ -0,0 +1,69 @@ +/* + * CanoKey QEMU device header. + * + * Copyright (c) 2021-2022 Canokeys.org + * Written by Hongren (Zenithal) Zheng + * + * This code is licensed under the Apache-2.0. + */ + +#ifndef CANOKEY_H +#define CANOKEY_H + +#include "hw/qdev-core.h" + +#define TYPE_CANOKEY "canokey" +#define CANOKEY(obj) \ + OBJECT_CHECK(CanoKeyState, (obj), TYPE_CANOKEY) + +/* + * State of Canokey (i.e. hw/canokey.c) + */ + +/* CTRL INTR BULK */ +#define CANOKEY_EP_NUM 3 +/* BULK/INTR IN can be up to 1352 bytes, e.g. get key info */ +#define CANOKEY_EP_IN_BUFFER_SIZE 2048 +/* BULK OUT can be up to 270 bytes, e.g. PIV import cert */ +#define CANOKEY_EP_OUT_BUFFER_SIZE 512 + +typedef enum { + CANOKEY_EP_IN_WAIT, + CANOKEY_EP_IN_READY, + CANOKEY_EP_IN_STALL +} CanoKeyEPState; + +typedef struct CanoKeyState { + USBDevice dev; + + /* IN packets from canokey device loop */ + uint8_t ep_in[CANOKEY_EP_NUM][CANOKEY_EP_IN_BUFFER_SIZE]; + /* + * See canokey_emu_transmit + * + * For large INTR IN, receive multiple data from canokey device loop + * in this case ep_in_size would increase with every call + */ + uint32_t ep_in_size[CANOKEY_EP_NUM]; + /* + * Used in canokey_handle_data + * for IN larger than p->iov.size, we would do multiple handle_data() + * + * The difference between ep_in_pos and ep_in_size: + * We first increase ep_in_size to fill ep_in buffer in device_loop, + * then use ep_in_pos to submit data from ep_in buffer in handle_data + */ + uint32_t ep_in_pos[CANOKEY_EP_NUM]; + CanoKeyEPState ep_in_state[CANOKEY_EP_NUM]; + + /* OUT pointer to canokey recv buffer */ + uint8_t *ep_out[CANOKEY_EP_NUM]; + uint32_t ep_out_size[CANOKEY_EP_NUM]; + /* For large BULK OUT, multiple write to ep_out is needed */ + uint8_t ep_out_buffer[CANOKEY_EP_NUM][CANOKEY_EP_OUT_BUFFER_SIZE]; + + /* Properties */ + char *file; /* canokey-file */ +} CanoKeyState; + +#endif /* CANOKEY_H */ From d37d0e0e855222b89008930b8675198ce74f8d4f Mon Sep 17 00:00:00 2001 From: "Hongren (Zenithal) Zheng" Date: Thu, 19 May 2022 20:38:30 +0800 Subject: [PATCH 05/15] hw/usb/canokey: Add trace events Signed-off-by: Hongren (Zenithal) Zheng Message-Id: Signed-off-by: Gerd Hoffmann --- hw/usb/canokey.c | 13 +++++++++++++ hw/usb/trace-events | 16 ++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c index 6cb8b7cdb0..4a08b1cbd7 100644 --- a/hw/usb/canokey.c +++ b/hw/usb/canokey.c @@ -14,6 +14,7 @@ #include "qapi/error.h" #include "hw/usb.h" #include "hw/qdev-properties.h" +#include "trace.h" #include "desc.h" #include "canokey.h" @@ -66,6 +67,7 @@ static const USBDesc desc_canokey = { */ int canokey_emu_stall_ep(void *base, uint8_t ep) { + trace_canokey_emu_stall_ep(ep); CanoKeyState *key = base; uint8_t ep_in = CANOKEY_EP_IN(ep); /* INTR IN has ep 129 */ key->ep_in_size[ep_in] = 0; @@ -75,6 +77,7 @@ int canokey_emu_stall_ep(void *base, uint8_t ep) int canokey_emu_set_address(void *base, uint8_t addr) { + trace_canokey_emu_set_address(addr); CanoKeyState *key = base; key->dev.addr = addr; return 0; @@ -83,6 +86,7 @@ int canokey_emu_set_address(void *base, uint8_t addr) int canokey_emu_prepare_receive( void *base, uint8_t ep, uint8_t *pbuf, uint16_t size) { + trace_canokey_emu_prepare_receive(ep, size); CanoKeyState *key = base; key->ep_out[ep] = pbuf; key->ep_out_size[ep] = size; @@ -92,6 +96,7 @@ int canokey_emu_prepare_receive( int canokey_emu_transmit( void *base, uint8_t ep, const uint8_t *pbuf, uint16_t size) { + trace_canokey_emu_transmit(ep, size); CanoKeyState *key = base; uint8_t ep_in = CANOKEY_EP_IN(ep); /* INTR IN has ep 129 */ memcpy(key->ep_in[ep_in] + key->ep_in_size[ep_in], @@ -125,6 +130,7 @@ uint32_t canokey_emu_get_rx_data_size(void *base, uint8_t ep) */ static void canokey_handle_reset(USBDevice *dev) { + trace_canokey_handle_reset(); CanoKeyState *key = CANOKEY(dev); for (int i = 0; i != CANOKEY_EP_NUM; ++i) { key->ep_in_state[i] = CANOKEY_EP_IN_WAIT; @@ -137,6 +143,7 @@ static void canokey_handle_reset(USBDevice *dev) static void canokey_handle_control(USBDevice *dev, USBPacket *p, int request, int value, int index, int length, uint8_t *data) { + trace_canokey_handle_control_setup(request, value, index, length); CanoKeyState *key = CANOKEY(dev); canokey_emu_setup(request, value, index, length); @@ -144,6 +151,7 @@ static void canokey_handle_control(USBDevice *dev, USBPacket *p, uint32_t dir_in = request & DeviceRequest; if (!dir_in) { /* OUT */ + trace_canokey_handle_control_out(); if (key->ep_out[0] != NULL) { memcpy(key->ep_out[0], data, length); } @@ -163,6 +171,7 @@ static void canokey_handle_control(USBDevice *dev, USBPacket *p, case CANOKEY_EP_IN_READY: memcpy(data, key->ep_in[0], key->ep_in_size[0]); p->actual_length = key->ep_in_size[0]; + trace_canokey_handle_control_in(p->actual_length); /* reset state */ key->ep_in_state[0] = CANOKEY_EP_IN_WAIT; key->ep_in_size[0] = 0; @@ -182,6 +191,7 @@ static void canokey_handle_data(USBDevice *dev, USBPacket *p) uint32_t out_len; switch (p->pid) { case USB_TOKEN_OUT: + trace_canokey_handle_data_out(ep_out, p->iov.size); usb_packet_copy(p, key->ep_out_buffer[ep_out], p->iov.size); out_pos = 0; while (out_pos != p->iov.size) { @@ -226,6 +236,7 @@ static void canokey_handle_data(USBDevice *dev, USBPacket *p) key->ep_in_size[ep_in] = 0; key->ep_in_pos[ep_in] = 0; } + trace_canokey_handle_data_in(ep_in, in_len); break; } break; @@ -237,6 +248,7 @@ static void canokey_handle_data(USBDevice *dev, USBPacket *p) static void canokey_realize(USBDevice *base, Error **errp) { + trace_canokey_realize(); CanoKeyState *key = CANOKEY(base); if (key->file == NULL) { @@ -260,6 +272,7 @@ static void canokey_realize(USBDevice *base, Error **errp) static void canokey_unrealize(USBDevice *base) { + trace_canokey_unrealize(); } static Property canokey_properties[] = { diff --git a/hw/usb/trace-events b/hw/usb/trace-events index 9773cb5330..914ca71668 100644 --- a/hw/usb/trace-events +++ b/hw/usb/trace-events @@ -345,3 +345,19 @@ usb_serial_set_baud(int bus, int addr, int baud) "dev %d:%u baud rate %d" usb_serial_set_data(int bus, int addr, int parity, int data, int stop) "dev %d:%u parity %c, data bits %d, stop bits %d" usb_serial_set_flow_control(int bus, int addr, int index) "dev %d:%u flow control %d" usb_serial_set_xonxoff(int bus, int addr, uint8_t xon, uint8_t xoff) "dev %d:%u xon 0x%x xoff 0x%x" + +# canokey.c +canokey_emu_stall_ep(uint8_t ep) "ep %d" +canokey_emu_set_address(uint8_t addr) "addr %d" +canokey_emu_prepare_receive(uint8_t ep, uint16_t size) "ep %d size %d" +canokey_emu_transmit(uint8_t ep, uint16_t size) "ep %d size %d" +canokey_thread_start(void) +canokey_thread_stop(void) +canokey_handle_reset(void) +canokey_handle_control_setup(int request, int value, int index, int length) "request 0x%04X value 0x%04X index 0x%04X length 0x%04X" +canokey_handle_control_out(void) +canokey_handle_control_in(int actual_len) "len %d" +canokey_handle_data_out(uint8_t ep_out, uint32_t out_len) "ep %d len %d" +canokey_handle_data_in(uint8_t ep_in, uint32_t in_len) "ep %d len %d" +canokey_realize(void) +canokey_unrealize(void) From 8caef8507809634fc70ecb1faa898a0edaf1ae5c Mon Sep 17 00:00:00 2001 From: "Hongren (Zenithal) Zheng" Date: Thu, 19 May 2022 20:38:57 +0800 Subject: [PATCH 06/15] meson: Add CanoKey Signed-off-by: Hongren (Zenithal) Zheng Message-Id: Signed-off-by: Gerd Hoffmann --- hw/usb/Kconfig | 5 +++++ hw/usb/meson.build | 5 +++++ meson.build | 6 ++++++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 3 +++ 5 files changed, 21 insertions(+) diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig index 53f8283ffd..ce4f433976 100644 --- a/hw/usb/Kconfig +++ b/hw/usb/Kconfig @@ -119,6 +119,11 @@ config USB_U2F default y depends on USB +config USB_CANOKEY + bool + default y + depends on USB + config IMX_USBPHY bool default y diff --git a/hw/usb/meson.build b/hw/usb/meson.build index de853d780d..793df42e21 100644 --- a/hw/usb/meson.build +++ b/hw/usb/meson.build @@ -63,6 +63,11 @@ if u2f.found() softmmu_ss.add(when: 'CONFIG_USB_U2F', if_true: [u2f, files('u2f-emulated.c')]) endif +# CanoKey +if canokey.found() + softmmu_ss.add(when: 'CONFIG_USB_CANOKEY', if_true: [canokey, files('canokey.c')]) +endif + # usb redirect if usbredir.found() usbredir_ss = ss.source_set() diff --git a/meson.build b/meson.build index 21cd949082..0c2e11ff07 100644 --- a/meson.build +++ b/meson.build @@ -1408,6 +1408,12 @@ if have_system method: 'pkg-config', kwargs: static_kwargs) endif +canokey = not_found +if have_system + canokey = dependency('canokey-qemu', required: get_option('canokey'), + method: 'pkg-config', + kwargs: static_kwargs) +endif usbredir = not_found if not get_option('usb_redir').auto() or have_system usbredir = dependency('libusbredirparser-0.5', required: get_option('usb_redir'), diff --git a/meson_options.txt b/meson_options.txt index 2de94af037..0e8197386b 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -189,6 +189,8 @@ option('spice_protocol', type : 'feature', value : 'auto', description: 'Spice protocol support') option('u2f', type : 'feature', value : 'auto', description: 'U2F emulation support') +option('canokey', type : 'feature', value : 'auto', + description: 'CanoKey support') option('usb_redir', type : 'feature', value : 'auto', description: 'libusbredir support') option('l2tpv3', type : 'feature', value : 'auto', diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index 00ea4d8cd1..1fc1d2e2c3 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -73,6 +73,7 @@ meson_options_help() { printf "%s\n" ' bpf eBPF support' printf "%s\n" ' brlapi brlapi character device driver' printf "%s\n" ' bzip2 bzip2 support for DMG images' + printf "%s\n" ' canokey CanoKey support' printf "%s\n" ' cap-ng cap_ng support' printf "%s\n" ' capstone Whether and how to find the capstone library' printf "%s\n" ' cloop cloop image format support' @@ -204,6 +205,8 @@ _meson_option_parse() { --disable-brlapi) printf "%s" -Dbrlapi=disabled ;; --enable-bzip2) printf "%s" -Dbzip2=enabled ;; --disable-bzip2) printf "%s" -Dbzip2=disabled ;; + --enable-canokey) printf "%s" -Dcanokey=enabled ;; + --disable-canokey) printf "%s" -Dcanokey=disabled ;; --enable-cap-ng) printf "%s" -Dcap_ng=enabled ;; --disable-cap-ng) printf "%s" -Dcap_ng=disabled ;; --enable-capstone) printf "%s" -Dcapstone=enabled ;; From 994e735c830f01e58710fdc01f5c527ed2ac7aba Mon Sep 17 00:00:00 2001 From: "Hongren (Zenithal) Zheng" Date: Thu, 19 May 2022 20:39:38 +0800 Subject: [PATCH 07/15] docs: Add CanoKey documentation Signed-off-by: Hongren (Zenithal) Zheng Message-Id: Signed-off-by: Gerd Hoffmann --- docs/system/device-emulation.rst | 1 + docs/system/devices/canokey.rst | 168 +++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 docs/system/devices/canokey.rst diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst index 3b729b920d..0506006056 100644 --- a/docs/system/device-emulation.rst +++ b/docs/system/device-emulation.rst @@ -92,3 +92,4 @@ Emulated Devices devices/vhost-user.rst devices/virtio-pmem.rst devices/vhost-user-rng.rst + devices/canokey.rst diff --git a/docs/system/devices/canokey.rst b/docs/system/devices/canokey.rst new file mode 100644 index 0000000000..169f99b8eb --- /dev/null +++ b/docs/system/devices/canokey.rst @@ -0,0 +1,168 @@ +.. _canokey: + +CanoKey QEMU +------------ + +CanoKey [1]_ is an open-source secure key with supports of + +* U2F / FIDO2 with Ed25519 and HMAC-secret +* OpenPGP Card V3.4 with RSA4096, Ed25519 and more [2]_ +* PIV (NIST SP 800-73-4) +* HOTP / TOTP +* NDEF + +All these platform-independent features are in canokey-core [3]_. + +For different platforms, CanoKey has different implementations, +including both hardware implementions and virtual cards: + +* CanoKey STM32 [4]_ +* CanoKey Pigeon [5]_ +* (virt-card) CanoKey USB/IP +* (virt-card) CanoKey FunctionFS + +In QEMU, yet another CanoKey virt-card is implemented. +CanoKey QEMU exposes itself as a USB device to the guest OS. + +With the same software configuration as a hardware key, +the guest OS can use all the functionalities of a secure key as if +there was actually an hardware key plugged in. + +CanoKey QEMU provides much convenience for debuging: + +* libcanokey-qemu supports debuging output thus developers can + inspect what happens inside a secure key +* CanoKey QEMU supports trace event thus event +* QEMU USB stack supports pcap thus USB packet between the guest + and key can be captured and analysed + +Then for developers: + +* For developers on software with secure key support (e.g. FIDO2, OpenPGP), + they can see what happens inside the secure key +* For secure key developers, USB packets between guest OS and CanoKey + can be easily captured and analysed + +Also since this is a virtual card, it can be easily used in CI for testing +on code coping with secure key. + +Building +======== + +libcanokey-qemu is required to use CanoKey QEMU. + +.. code-block:: shell + + git clone https://github.com/canokeys/canokey-qemu + mkdir canokey-qemu/build + pushd canokey-qemu/build + +If you want to install libcanokey-qemu in a different place, +add ``-DCMAKE_INSTALL_PREFIX=/path/to/your/place`` to cmake below. + +.. code-block:: shell + + cmake .. + make + make install # may need sudo + popd + +Then configuring and building: + +.. code-block:: shell + + # depending on your env, lib/pkgconfig can be lib64/pkgconfig + export PKG_CONFIG_PATH=/path/to/your/place/lib/pkgconfig:$PKG_CONFIG_PATH + ./configure --enable-canokey && make + +Using CanoKey QEMU +================== + +CanoKey QEMU stores all its data on a file of the host specified by the argument +when invoking qemu. + +.. parsed-literal:: + + |qemu_system| -usb -device canokey,file=$HOME/.canokey-file + +Note: you should keep this file carefully as it may contain your private key! + +The first time when the file is used, it is created and initialized by CanoKey, +afterwards CanoKey QEMU would just read this file. + +After the guest OS boots, you can check that there is a USB device. + +For example, If the guest OS is an Linux machine. You may invoke lsusb +and find CanoKey QEMU there: + +.. code-block:: shell + + $ lsusb + Bus 001 Device 002: ID 20a0:42d4 Clay Logic CanoKey QEMU + +You may setup the key as guided in [6]_. The console for the key is at [7]_. + +Debuging +======== + +CanoKey QEMU consists of two parts, ``libcanokey-qemu.so`` and ``canokey.c``, +the latter of which resides in QEMU. The former provides core functionality +of a secure key while the latter provides platform-dependent functions: +USB packet handling. + +If you want to trace what happens inside the secure key, when compiling +libcanokey-qemu, you should add ``-DQEMU_DEBUG_OUTPUT=ON`` in cmake command +line: + +.. code-block:: shell + + cmake .. -DQEMU_DEBUG_OUTPUT=ON + +If you want to trace events happened in canokey.c, use + +.. parsed-literal:: + + |qemu_system| --trace "canokey_*" \\ + -usb -device canokey,file=$HOME/.canokey-file + +If you want to capture USB packets between the guest and the host, you can: + +.. parsed-literal:: + + |qemu_system| -usb -device canokey,file=$HOME/.canokey-file,pcap=key.pcap + +Limitations +=========== + +Currently libcanokey-qemu.so has dozens of global variables as it was originally +designed for embedded systems. Thus one qemu instance can not have +multiple CanoKey QEMU running, namely you can not + +.. parsed-literal:: + + |qemu_system| -usb -device canokey,file=$HOME/.canokey-file \\ + -device canokey,file=$HOME/.canokey-file2 + +Also, there is no lock on canokey-file, thus two CanoKey QEMU instance +can not read one canokey-file at the same time. + +Another limitation is that this device is not compatible with ``qemu-xhci``, +in that this device would hang when there are FIDO2 packets (traffic on +interrupt endpoints). If you do not use FIDO2 then it works as intended, +but for full functionality you should use old uhci/ehci bus and attach canokey +to it, for example + +.. parsed-literal:: + + |qemu_system| -device piix3-usb-uhci,id=uhci -device canokey,bus=uhci.0 + +References +========== + +.. [1] ``_ +.. [2] ``_ +.. [3] ``_ +.. [4] ``_ +.. [5] ``_ +.. [6] ``_ +.. [7] ``_ From adaf4d2e84d30e559327917b0725f98f0d5b8cd4 Mon Sep 17 00:00:00 2001 From: "Hongren (Zenithal) Zheng" Date: Thu, 19 May 2022 20:40:03 +0800 Subject: [PATCH 08/15] docs/system/devices/usb: Add CanoKey to USB devices examples Signed-off-by: Hongren (Zenithal) Zheng Message-Id: Signed-off-by: Gerd Hoffmann --- docs/system/devices/usb.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/system/devices/usb.rst b/docs/system/devices/usb.rst index afb7d6c226..872d916758 100644 --- a/docs/system/devices/usb.rst +++ b/docs/system/devices/usb.rst @@ -199,6 +199,10 @@ option or the ``device_add`` monitor command. Available devices are: ``u2f-{emulated,passthru}`` Universal Second Factor device +``canokey`` + An Open-source Secure Key implementing FIDO2, OpenPGP, PIV and more. + For more information, see :ref:`canokey`. + Physical port addressing ^^^^^^^^^^^^^^^^^^^^^^^^ From 5028d66cb2fb5ec6c2997b1ab274230ffdd57bbc Mon Sep 17 00:00:00 2001 From: "Hongren (Zenithal) Zheng" Date: Thu, 19 May 2022 20:40:55 +0800 Subject: [PATCH 09/15] MAINTAINERS: add myself as CanoKey maintainer Signed-off-by: Hongren (Zenithal) Zheng Message-Id: Signed-off-by: Gerd Hoffmann --- MAINTAINERS | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0df25ed4b0..4cf6174f9f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2427,6 +2427,14 @@ F: hw/intc/s390_flic*.c F: include/hw/s390x/s390_flic.h L: qemu-s390x@nongnu.org +CanoKey +M: Hongren (Zenithal) Zheng +S: Maintained +R: Canokeys.org +F: hw/usb/canokey.c +F: hw/usb/canokey.h +F: docs/system/devices/canokey.rst + Subsystems ---------- Overall Audio backends From f471e8b060798f26a7fc339c6152f82f22a7b33d Mon Sep 17 00:00:00 2001 From: Arnout Engelen Date: Sun, 8 May 2022 17:32:22 +0200 Subject: [PATCH 10/15] hw/usb/hcd-ehci: fix writeback order The 'active' bit passes control over a qTD between the guest and the controller: set to 1 by guest to enable execution by the controller, and the controller sets it to '0' to hand back control to the guest. ehci_state_writeback write two dwords to main memory using DMA: the third dword of the qTD (containing dt, total bytes to transfer, cpage, cerr and status) and the fourth dword of the qTD (containing the offset). This commit makes sure the fourth dword is written before the third, avoiding a race condition where a new offset written into the qTD by the guest after it observed the status going to go to '0' gets overwritten by a 'late' DMA writeback of the previous offset. This race condition could lead to 'cpage out of range (5)' errors, and reproduced by: ./qemu-system-x86_64 -enable-kvm -bios $SEABIOS/bios.bin -m 4096 -device usb-ehci -blockdev driver=file,read-only=on,filename=/home/aengelen/Downloads/openSUSE-Tumbleweed-DVD-i586-Snapshot20220428-Media.iso,node-name=iso -device usb-storage,drive=iso,bootindex=0 -chardev pipe,id=shell,path=/tmp/pipe -device virtio-serial -device virtconsole,chardev=shell -device virtio-rng-pci -serial mon:stdio -nographic (press a key, select 'Installation' (2), and accept the default values. On my machine the 'cpage out of range' is reproduced while loading the Linux Kernel about once per 7 attempts. With the fix in this commit it no longer fails) This problem was previously reported as a seabios problem in https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/OUTHT5ISSQJGXPNTUPY3O5E5EPZJCHM3/ and as a nixos CI build failure in https://github.com/NixOS/nixpkgs/issues/170803 Signed-off-by: Arnout Engelen Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 33a8a377bd..d4da8dcb8d 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2011,7 +2011,10 @@ static int ehci_state_writeback(EHCIQueue *q) ehci_trace_qtd(q, NLPTR_GET(p->qtdaddr), (EHCIqtd *) &q->qh.next_qtd); qtd = (uint32_t *) &q->qh.next_qtd; addr = NLPTR_GET(p->qtdaddr); - put_dwords(q->ehci, addr + 2 * sizeof(uint32_t), qtd + 2, 2); + /* First write back the offset */ + put_dwords(q->ehci, addr + 3 * sizeof(uint32_t), qtd + 3, 1); + /* Then write back the token, clearing the 'active' bit */ + put_dwords(q->ehci, addr + 2 * sizeof(uint32_t), qtd + 2, 1); ehci_free_packet(p); /* From 0631d4b448454ae8a1ab091c447e3f71ab6e088a Mon Sep 17 00:00:00 2001 From: Joelle van Dyne Date: Fri, 6 May 2022 21:18:50 -0700 Subject: [PATCH 11/15] usbredir: avoid queuing hello packet on snapshot restore When launching QEMU with "-loadvm", usbredir_create_parser() should avoid setting up the hello packet (just as with "-incoming". On the latest version of libusbredir, usbredirparser_unserialize() will return error if the parser is not "pristine." Signed-off-by: Joelle van Dyne Message-Id: <20220507041850.98716-1-j@getutm.app> Signed-off-by: Gerd Hoffmann --- hw/usb/redirect.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index fd7df599bc..1bd30efc3e 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -1280,7 +1280,8 @@ static void usbredir_create_parser(USBRedirDevice *dev) } #endif - if (runstate_check(RUN_STATE_INMIGRATE)) { + if (runstate_check(RUN_STATE_INMIGRATE) || + runstate_check(RUN_STATE_PRELAUNCH)) { flags |= usbredirparser_fl_no_hello; } usbredirparser_init(dev->parser, VERSION, caps, USB_REDIR_CAPS_SIZE, From 45e64ab63d7deb77b9fd1d2a3d43fee5cb17a5b5 Mon Sep 17 00:00:00 2001 From: Dongwon Kim Date: Thu, 5 May 2022 14:40:30 -0700 Subject: [PATCH 12/15] virtio-gpu: update done only on the scanout associated with rect It only needs to update the scanouts containing the rect area coming with the resource-flush request from the guest. Cc: Gerd Hoffmann Cc: Vivek Kasireddy Signed-off-by: Dongwon Kim Message-Id: <20220505214030.4261-1-dongwon.kim@intel.com> Signed-off-by: Gerd Hoffmann --- hw/display/virtio-gpu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index cd4a56056f..55c6dd5763 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -514,6 +514,9 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g, for (i = 0; i < g->parent_obj.conf.max_outputs; i++) { scanout = &g->parent_obj.scanout[i]; if (scanout->resource_id == res->resource_id && + rf.r.x >= scanout->x && rf.r.y >= scanout->y && + rf.r.x + rf.r.width <= scanout->x + scanout->width && + rf.r.y + rf.r.height <= scanout->y + scanout->height && console_has_gl(scanout->con)) { dpy_gl_update(scanout->con, 0, 0, scanout->width, scanout->height); From 362239c05f28e8185d00d66884729a726ec4d226 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 26 Feb 2022 20:55:14 +0900 Subject: [PATCH 13/15] ui/console: Do not return a value with ui_info The returned value is not used and misleading. Signed-off-by: Akihiko Odaki Message-Id: <20220226115516.59830-2-akihiko.odaki@gmail.com> Signed-off-by: Gerd Hoffmann --- hw/display/virtio-gpu-base.c | 6 +++--- hw/display/virtio-vga.c | 5 ++--- hw/vfio/display.c | 8 +++----- include/ui/console.h | 2 +- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c index 790cec333c..b21d6e5b0b 100644 --- a/hw/display/virtio-gpu-base.c +++ b/hw/display/virtio-gpu-base.c @@ -69,12 +69,12 @@ static void virtio_gpu_notify_event(VirtIOGPUBase *g, uint32_t event_type) virtio_notify_config(&g->parent_obj); } -static int virtio_gpu_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info) +static void virtio_gpu_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info) { VirtIOGPUBase *g = opaque; if (idx >= g->conf.max_outputs) { - return -1; + return; } g->req_state[idx].x = info->xoff; @@ -92,7 +92,7 @@ static int virtio_gpu_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info) /* send event to guest */ virtio_gpu_notify_event(g, VIRTIO_GPU_EVENT_DISPLAY); - return 0; + return; } static void diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c index c206b5da38..4dcb34c4a7 100644 --- a/hw/display/virtio-vga.c +++ b/hw/display/virtio-vga.c @@ -47,15 +47,14 @@ static void virtio_vga_base_text_update(void *opaque, console_ch_t *chardata) } } -static int virtio_vga_base_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info) +static void virtio_vga_base_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info) { VirtIOVGABase *vvga = opaque; VirtIOGPUBase *g = vvga->vgpu; if (g->hw_ops->ui_info) { - return g->hw_ops->ui_info(g, idx, info); + g->hw_ops->ui_info(g, idx, info); } - return -1; } static void virtio_vga_base_gl_block(void *opaque, bool block) diff --git a/hw/vfio/display.c b/hw/vfio/display.c index 89bc90508f..78f4d82c1c 100644 --- a/hw/vfio/display.c +++ b/hw/vfio/display.c @@ -106,14 +106,14 @@ err: return; } -static int vfio_display_edid_ui_info(void *opaque, uint32_t idx, - QemuUIInfo *info) +static void vfio_display_edid_ui_info(void *opaque, uint32_t idx, + QemuUIInfo *info) { VFIOPCIDevice *vdev = opaque; VFIODisplay *dpy = vdev->dpy; if (!dpy->edid_regs) { - return 0; + return; } if (info->width && info->height) { @@ -121,8 +121,6 @@ static int vfio_display_edid_ui_info(void *opaque, uint32_t idx, } else { vfio_display_edid_update(vdev, false, 0, 0); } - - return 0; } static void vfio_display_edid_init(VFIOPCIDevice *vdev) diff --git a/include/ui/console.h b/include/ui/console.h index c44b28a972..642d6f5248 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -432,7 +432,7 @@ typedef struct GraphicHwOps { bool gfx_update_async; /* if true, calls graphic_hw_update_done() */ void (*text_update)(void *opaque, console_ch_t *text); void (*update_interval)(void *opaque, uint64_t interval); - int (*ui_info)(void *opaque, uint32_t head, QemuUIInfo *info); + void (*ui_info)(void *opaque, uint32_t head, QemuUIInfo *info); void (*gl_block)(void *opaque, bool block); } GraphicHwOps; From aeffd071ed818fa83c723fe8e2715144ee8385c7 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 26 Feb 2022 20:55:15 +0900 Subject: [PATCH 14/15] ui: Deliver refresh rate via QemuUIInfo This change adds a new member, refresh_rate to QemuUIInfo in include/ui/console.h. It represents the refresh rate of the physical display backend, and it is more appropriate than GUI update interval as the refresh rate which the emulated device reports: - sdl may set GUI update interval shorter than the refresh rate of the physical display to respond to user-generated events. - sdl and vnc aggressively changes GUI update interval, but a guests is typically not designed to respond to frequent refresh rate changes, or frequent "display mode" changes in general. The frequency of refresh rate changes of the physical display backend matches better to the guest's expectation. QemuUIInfo also has other members representing "display mode", which makes it suitable for refresh rate representation. It has a throttling of update notifications, and prevents frequent changes of the display mode. Signed-off-by: Akihiko Odaki Message-Id: <20220226115516.59830-3-akihiko.odaki@gmail.com> Signed-off-by: Gerd Hoffmann --- hw/display/xenfb.c | 14 +++++++++++--- include/ui/console.h | 2 +- include/ui/gtk.h | 2 +- ui/console.c | 6 ------ ui/gtk-egl.c | 4 ++-- ui/gtk-gl-area.c | 3 +-- ui/gtk.c | 45 +++++++++++++++++++++++++------------------- 7 files changed, 42 insertions(+), 34 deletions(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index cea10fe3c7..50857cd97a 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -777,16 +777,24 @@ static void xenfb_update(void *opaque) xenfb->up_fullscreen = 0; } -static void xenfb_update_interval(void *opaque, uint64_t interval) +static void xenfb_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info) { struct XenFB *xenfb = opaque; + uint32_t refresh_rate; if (xenfb->feature_update) { #ifdef XENFB_TYPE_REFRESH_PERIOD if (xenfb_queue_full(xenfb)) { return; } - xenfb_send_refresh_period(xenfb, interval); + + refresh_rate = info->refresh_rate; + if (!refresh_rate) { + refresh_rate = 75; + } + + /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */ + xenfb_send_refresh_period(xenfb, 1000 * 1000 / refresh_rate); #endif } } @@ -983,5 +991,5 @@ struct XenDevOps xen_framebuffer_ops = { static const GraphicHwOps xenfb_ops = { .invalidate = xenfb_invalidate, .gfx_update = xenfb_update, - .update_interval = xenfb_update_interval, + .ui_info = xenfb_ui_info, }; diff --git a/include/ui/console.h b/include/ui/console.h index 642d6f5248..b64d824360 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -139,6 +139,7 @@ typedef struct QemuUIInfo { int yoff; uint32_t width; uint32_t height; + uint32_t refresh_rate; } QemuUIInfo; /* cursor data format is 32bit RGBA */ @@ -431,7 +432,6 @@ typedef struct GraphicHwOps { void (*gfx_update)(void *opaque); bool gfx_update_async; /* if true, calls graphic_hw_update_done() */ void (*text_update)(void *opaque, console_ch_t *text); - void (*update_interval)(void *opaque, uint64_t interval); void (*ui_info)(void *opaque, uint32_t head, QemuUIInfo *info); void (*gl_block)(void *opaque, bool block); } GraphicHwOps; diff --git a/include/ui/gtk.h b/include/ui/gtk.h index 101b147d1b..ae0f53740d 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -155,7 +155,7 @@ extern bool gtk_use_gl_area; /* ui/gtk.c */ void gd_update_windowsize(VirtualConsole *vc); -int gd_monitor_update_interval(GtkWidget *widget); +void gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget *widget); void gd_hw_gl_flushed(void *vc); /* ui/gtk-egl.c */ diff --git a/ui/console.c b/ui/console.c index 36c80cd1de..9331b85203 100644 --- a/ui/console.c +++ b/ui/console.c @@ -160,7 +160,6 @@ static void gui_update(void *opaque) uint64_t dcl_interval; DisplayState *ds = opaque; DisplayChangeListener *dcl; - QemuConsole *con; ds->refreshing = true; dpy_refresh(ds); @@ -175,11 +174,6 @@ static void gui_update(void *opaque) } if (ds->update_interval != interval) { ds->update_interval = interval; - QTAILQ_FOREACH(con, &consoles, next) { - if (con->hw_ops->update_interval) { - con->hw_ops->update_interval(con->hw, interval); - } - } trace_console_refresh(interval); } ds->last_update = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c index e3bd4bc274..b5bffbab25 100644 --- a/ui/gtk-egl.c +++ b/ui/gtk-egl.c @@ -140,8 +140,8 @@ void gd_egl_refresh(DisplayChangeListener *dcl) { VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl); - vc->gfx.dcl.update_interval = gd_monitor_update_interval( - vc->window ? vc->window : vc->gfx.drawing_area); + gd_update_monitor_refresh_rate( + vc, vc->window ? vc->window : vc->gfx.drawing_area); if (!vc->gfx.esurface) { gd_egl_init(vc); diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index 2e0129c28c..682638a197 100644 --- a/ui/gtk-gl-area.c +++ b/ui/gtk-gl-area.c @@ -121,8 +121,7 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl) { VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl); - vc->gfx.dcl.update_interval = gd_monitor_update_interval( - vc->window ? vc->window : vc->gfx.drawing_area); + gd_update_monitor_refresh_rate(vc, vc->window ? vc->window : vc->gfx.drawing_area); if (!vc->gfx.gls) { if (!gtk_widget_get_realized(vc->gfx.drawing_area)) { diff --git a/ui/gtk.c b/ui/gtk.c index c57c36749e..2a791dd2aa 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -710,11 +710,20 @@ static gboolean gd_window_close(GtkWidget *widget, GdkEvent *event, return TRUE; } -static void gd_set_ui_info(VirtualConsole *vc, gint width, gint height) +static void gd_set_ui_refresh_rate(VirtualConsole *vc, int refresh_rate) { QemuUIInfo info; - memset(&info, 0, sizeof(info)); + info = *dpy_get_ui_info(vc->gfx.dcl.con); + info.refresh_rate = refresh_rate; + dpy_set_ui_info(vc->gfx.dcl.con, &info, true); +} + +static void gd_set_ui_size(VirtualConsole *vc, gint width, gint height) +{ + QemuUIInfo info; + + info = *dpy_get_ui_info(vc->gfx.dcl.con); info.width = width; info.height = height; dpy_set_ui_info(vc->gfx.dcl.con, &info, true); @@ -738,33 +747,32 @@ static void gd_resize_event(GtkGLArea *area, { VirtualConsole *vc = (void *)opaque; - gd_set_ui_info(vc, width, height); + gd_set_ui_size(vc, width, height); } #endif -/* - * If available, return the update interval of the monitor in ms, - * else return 0 (the default update interval). - */ -int gd_monitor_update_interval(GtkWidget *widget) +void gd_update_monitor_refresh_rate(VirtualConsole *vc, GtkWidget *widget) { #ifdef GDK_VERSION_3_22 GdkWindow *win = gtk_widget_get_window(widget); + int refresh_rate; if (win) { GdkDisplay *dpy = gtk_widget_get_display(widget); GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win); - int refresh_rate = gdk_monitor_get_refresh_rate(monitor); /* [mHz] */ - - if (refresh_rate) { - /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */ - return MIN(1000 * 1000 / refresh_rate, - GUI_REFRESH_INTERVAL_DEFAULT); - } + refresh_rate = gdk_monitor_get_refresh_rate(monitor); /* [mHz] */ + } else { + refresh_rate = 0; } + + gd_set_ui_refresh_rate(vc, refresh_rate); + + /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */ + vc->gfx.dcl.update_interval = refresh_rate ? + MIN(1000 * 1000 / refresh_rate, GUI_REFRESH_INTERVAL_DEFAULT) : + GUI_REFRESH_INTERVAL_DEFAULT; #endif - return 0; } static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque) @@ -801,8 +809,7 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque) return FALSE; } - vc->gfx.dcl.update_interval = - gd_monitor_update_interval(vc->window ? vc->window : s->window); + gd_update_monitor_refresh_rate(vc, vc->window ? vc->window : s->window); fbw = surface_width(vc->gfx.ds); fbh = surface_height(vc->gfx.ds); @@ -1691,7 +1698,7 @@ static gboolean gd_configure(GtkWidget *widget, { VirtualConsole *vc = opaque; - gd_set_ui_info(vc, cfg->width, cfg->height); + gd_set_ui_size(vc, cfg->width, cfg->height); return FALSE; } From b95b56311a0890da0c9f7fc624529c3d7f8dbce0 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 26 Feb 2022 20:55:16 +0900 Subject: [PATCH 15/15] virtio-gpu: Respect UI refresh rate for EDID Signed-off-by: Akihiko Odaki Message-Id: <20220226115516.59830-4-akihiko.odaki@gmail.com> Signed-off-by: Gerd Hoffmann --- hw/display/virtio-gpu-base.c | 1 + hw/display/virtio-gpu.c | 1 + include/hw/virtio/virtio-gpu.h | 1 + 3 files changed, 3 insertions(+) diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c index b21d6e5b0b..a29f191aa8 100644 --- a/hw/display/virtio-gpu-base.c +++ b/hw/display/virtio-gpu-base.c @@ -79,6 +79,7 @@ static void virtio_gpu_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info) g->req_state[idx].x = info->xoff; g->req_state[idx].y = info->yoff; + g->req_state[idx].refresh_rate = info->refresh_rate; g->req_state[idx].width = info->width; g->req_state[idx].height = info->height; g->req_state[idx].width_mm = info->width_mm; diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 55c6dd5763..20cc703dcc 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -217,6 +217,7 @@ virtio_gpu_generate_edid(VirtIOGPU *g, int scanout, .height_mm = b->req_state[scanout].height_mm, .prefx = b->req_state[scanout].width, .prefy = b->req_state[scanout].height, + .refresh_rate = b->req_state[scanout].refresh_rate, }; edid->size = cpu_to_le32(sizeof(edid->edid)); diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index afff9e158e..2e28507efe 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -80,6 +80,7 @@ struct virtio_gpu_scanout { struct virtio_gpu_requested_state { uint16_t width_mm, height_mm; uint32_t width, height; + uint32_t refresh_rate; int x, y; };