From 42c93917f7829190fcae05c5c0827d0c187d7713 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Thu, 23 Dec 2021 12:54:43 +0300 Subject: [PATCH 01/35] hw/usb: pacify xhciwmi.exe warning xhciwmi.exe is used inside Windows 2022 SVVP tests. This tool called as 'xhciwmi.exe --verify' reports that 'The firmware loaded on this controller has known bugs and/or compatibility issues'. This is just a warning but there is no particular sense to ignore it. This patch just pacifies the tool. There is a big question whether this change should be put using machine type mechanics, but at my opinion this would be an overkill. Signed-off-by: Denis V. Lunev Tested-by: Pavel Polozov CC: Yan Vugenfirer CC: Gerd Hoffmann Reviewed-by: Yan Vugenfirer Message-Id: <20211223095443.130276-1-den@openvz.org> Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 14bdb89676..0cd0a5e540 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2523,7 +2523,7 @@ static void xhci_process_commands(XHCIState *xhci) case CR_VENDOR_NEC_FIRMWARE_REVISION: if (xhci->nec_quirks) { event.type = 48; /* NEC reply */ - event.length = 0x3025; + event.length = 0x3034; } else { event.ccode = CC_TRB_ERROR; } From 7c204e96384b8e03b917fd30abf3bc96da237990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sat, 22 Jan 2022 15:06:19 +0100 Subject: [PATCH 02/35] hw/usb/dev-mtp: create directories with a+x mode mask MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Current code creates directories with mode 0644. Even the creator can't create files in the new directory. Set all x mode flags in variable mask and clear all x mode flags in function open() to preserve the current open mode. Signed-off-by: Volker Rümelin Message-Id: <20220122140619.7514-1-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- hw/usb/dev-mtp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c index 1e6ac76bef..e6b77a2a94 100644 --- a/hw/usb/dev-mtp.c +++ b/hw/usb/dev-mtp.c @@ -1607,7 +1607,7 @@ static void usb_mtp_write_data(MTPState *s, uint32_t handle) usb_mtp_object_lookup(s, s->dataset.parent_handle); char *path = NULL; uint64_t rc; - mode_t mask = 0644; + mode_t mask = 0755; int ret = 0; assert(d != NULL); @@ -1635,7 +1635,7 @@ static void usb_mtp_write_data(MTPState *s, uint32_t handle) } d->fd = open(path, O_CREAT | O_WRONLY | - O_CLOEXEC | O_NOFOLLOW, mask); + O_CLOEXEC | O_NOFOLLOW, mask & 0666); if (d->fd == -1) { ret = 1; goto done; From 6e821e5084fb1169d653c835b3819acf94e8b52c Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Tue, 25 Jan 2022 14:33:20 +0100 Subject: [PATCH 03/35] usb/ohci: Move trace point and log ep number to help debugging Signed-off-by: BALATON Zoltan Message-Id: <4e3a05a64b5029a88654eab9a873fb45ac80b1a7.1643117600.git.balaton@eik.bme.hu> Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 18 +++++++++--------- hw/usb/trace-events | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index a93d6b2e98..f915cc5473 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -1033,21 +1033,21 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed) ohci->async_td = 0; ohci->async_complete = false; } else { - if (ohci->async_td) { - /* ??? The hardware should allow one active packet per - endpoint. We only allow one active packet per controller. - This should be sufficient as long as devices respond in a - timely manner. - */ - trace_usb_ohci_td_too_many_pending(); - return 1; - } dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA)); if (dev == NULL) { trace_usb_ohci_td_dev_error(); return 1; } ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN)); + if (ohci->async_td) { + /* ??? The hardware should allow one active packet per + endpoint. We only allow one active packet per controller. + This should be sufficient as long as devices respond in a + timely manner. + */ + trace_usb_ohci_td_too_many_pending(ep->nr); + return 1; + } usb_packet_setup(&ohci->usb_packet, pid, ep, 0, addr, !flag_r, OHCI_BM(td.flags, TD_DI) == 0); usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, pktlen); diff --git a/hw/usb/trace-events b/hw/usb/trace-events index b8287b63f1..9773cb5330 100644 --- a/hw/usb/trace-events +++ b/hw/usb/trace-events @@ -51,7 +51,7 @@ usb_ohci_td_skip_async(void) "" usb_ohci_td_pkt_hdr(uint32_t addr, int64_t pktlen, int64_t len, const char *s, int flag_r, uint32_t cbp, uint32_t be) " TD @ 0x%.8x %" PRId64 " of %" PRId64 " bytes %s r=%d cbp=0x%.8x be=0x%.8x" usb_ohci_td_pkt_short(const char *dir, const char *buf) "%s data: %s" usb_ohci_td_pkt_full(const char *dir, const char *buf) "%s data: %s" -usb_ohci_td_too_many_pending(void) "" +usb_ohci_td_too_many_pending(int ep) "ep=%d" usb_ohci_td_packet_status(int status) "status=%d" usb_ohci_ed_read_error(uint32_t addr) "ED read error at 0x%x" usb_ohci_ed_pkt(uint32_t cur, int h, int c, uint32_t head, uint32_t tail, uint32_t next) "ED @ 0x%.8x h=%u c=%u\n head=0x%.8x tailp=0x%.8x next=0x%.8x" From ae310557f4ec635dadb5a0857fb9bd592016919c Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Tue, 25 Jan 2022 14:33:20 +0100 Subject: [PATCH 04/35] usb/ohci: Move cancelling async packet to ohci_stop_endpoints() This is always done before calling this function so remove duplicated code and do it within the function at one place. Signed-off-by: BALATON Zoltan Message-Id: Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index f915cc5473..6d762973eb 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -369,6 +369,10 @@ void ohci_stop_endpoints(OHCIState *ohci) USBDevice *dev; int i, j; + if (ohci->async_td) { + usb_cancel_packet(&ohci->usb_packet); + ohci->async_td = 0; + } for (i = 0; i < ohci->num_ports; i++) { dev = ohci->rhport[i].port.dev; if (dev && dev->attached) { @@ -398,10 +402,6 @@ static void ohci_roothub_reset(OHCIState *ohci) usb_port_reset(&port->port); } } - if (ohci->async_td) { - usb_cancel_packet(&ohci->usb_packet); - ohci->async_td = 0; - } ohci_stop_endpoints(ohci); } @@ -1277,10 +1277,6 @@ static void ohci_frame_boundary(void *opaque) /* Cancel all pending packets if either of the lists has been disabled. */ if (ohci->old_ctl & (~ohci->ctl) & (OHCI_CTL_BLE | OHCI_CTL_CLE)) { - if (ohci->async_td) { - usb_cancel_packet(&ohci->usb_packet); - ohci->async_td = 0; - } ohci_stop_endpoints(ohci); } ohci->old_ctl = ohci->ctl; From 37bf0654b8e6195e2255e7d5f737a66b0ef4d83f Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Tue, 25 Jan 2022 14:33:20 +0100 Subject: [PATCH 05/35] usb/ohci: Move USBPortOps related functions together MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This also allows removing two forward declarations Signed-off-by: BALATON Zoltan Reviewed-by: Philippe Mathieu-Daudé Message-Id: <9fd730375c4cad0b11163631660d68711d3fc13f.1643117600.git.balaton@eik.bme.hu> Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 205 +++++++++++++++++++++++----------------------- 1 file changed, 101 insertions(+), 104 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 6d762973eb..190f5a8aba 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -58,8 +58,6 @@ struct ohci_hcca { #define ED_WBACK_OFFSET offsetof(struct ohci_ed, head) #define ED_WBACK_SIZE 4 -static void ohci_async_cancel_device(OHCIState *ohci, USBDevice *dev); - /* Bitfields for the first word of an Endpoint Desciptor. */ #define OHCI_ED_FA_SHIFT 0 #define OHCI_ED_FA_MASK (0x7f<opaque; - OHCIPort *port = &s->rhport[port1->index]; - uint32_t old_state = port->ctrl; - - /* set connect status */ - port->ctrl |= OHCI_PORT_CCS | OHCI_PORT_CSC; - - /* update speed */ - if (port->port.dev->speed == USB_SPEED_LOW) { - port->ctrl |= OHCI_PORT_LSDA; - } else { - port->ctrl &= ~OHCI_PORT_LSDA; - } - - /* notify of remote-wakeup */ - if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) { - ohci_set_interrupt(s, OHCI_INTR_RD); - } - - trace_usb_ohci_port_attach(port1->index); - - if (old_state != port->ctrl) { - ohci_set_interrupt(s, OHCI_INTR_RHSC); - } -} - -static void ohci_detach(USBPort *port1) -{ - OHCIState *s = port1->opaque; - OHCIPort *port = &s->rhport[port1->index]; - uint32_t old_state = port->ctrl; - - ohci_async_cancel_device(s, port1->dev); - - /* set connect status */ - if (port->ctrl & OHCI_PORT_CCS) { - port->ctrl &= ~OHCI_PORT_CCS; - port->ctrl |= OHCI_PORT_CSC; - } - /* disable port */ - if (port->ctrl & OHCI_PORT_PES) { - port->ctrl &= ~OHCI_PORT_PES; - port->ctrl |= OHCI_PORT_PESC; - } - trace_usb_ohci_port_detach(port1->index); - - if (old_state != port->ctrl) { - ohci_set_interrupt(s, OHCI_INTR_RHSC); - } -} - -static void ohci_wakeup(USBPort *port1) -{ - OHCIState *s = port1->opaque; - OHCIPort *port = &s->rhport[port1->index]; - uint32_t intr = 0; - if (port->ctrl & OHCI_PORT_PSS) { - trace_usb_ohci_port_wakeup(port1->index); - port->ctrl |= OHCI_PORT_PSSC; - port->ctrl &= ~OHCI_PORT_PSS; - intr = OHCI_INTR_RHSC; - } - /* Note that the controller can be suspended even if this port is not */ - if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) { - trace_usb_ohci_remote_wakeup(s->name); - /* This is the one state transition the controller can do by itself */ - s->ctl &= ~OHCI_CTL_HCFS; - s->ctl |= OHCI_USB_RESUME; - /* In suspend mode only ResumeDetected is possible, not RHSC: - * see the OHCI spec 5.1.2.3. - */ - intr = OHCI_INTR_RD; - } - ohci_set_interrupt(s, intr); -} - -static void ohci_child_detach(USBPort *port1, USBDevice *child) -{ - OHCIState *s = port1->opaque; - - ohci_async_cancel_device(s, child); -} - static USBDevice *ohci_find_device(OHCIState *ohci, uint8_t addr) { USBDevice *dev; @@ -634,17 +546,6 @@ static int ohci_copy_iso_td(OHCIState *ohci, return 0; } -static void ohci_process_lists(OHCIState *ohci, int completion); - -static void ohci_async_complete_packet(USBPort *port, USBPacket *packet) -{ - OHCIState *ohci = container_of(packet, OHCIState, usb_packet); - - trace_usb_ohci_async_complete(); - ohci->async_complete = true; - ohci_process_lists(ohci, 1); -} - #define USUB(a, b) ((int16_t)((uint16_t)(a) - (uint16_t)(b))) static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, @@ -1789,6 +1690,41 @@ static void ohci_mem_write(void *opaque, } } +static const MemoryRegionOps ohci_mem_ops = { + .read = ohci_mem_read, + .write = ohci_mem_write, + .endianness = DEVICE_LITTLE_ENDIAN, +}; + +/* USBPortOps */ +static void ohci_attach(USBPort *port1) +{ + OHCIState *s = port1->opaque; + OHCIPort *port = &s->rhport[port1->index]; + uint32_t old_state = port->ctrl; + + /* set connect status */ + port->ctrl |= OHCI_PORT_CCS | OHCI_PORT_CSC; + + /* update speed */ + if (port->port.dev->speed == USB_SPEED_LOW) { + port->ctrl |= OHCI_PORT_LSDA; + } else { + port->ctrl &= ~OHCI_PORT_LSDA; + } + + /* notify of remote-wakeup */ + if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) { + ohci_set_interrupt(s, OHCI_INTR_RD); + } + + trace_usb_ohci_port_attach(port1->index); + + if (old_state != port->ctrl) { + ohci_set_interrupt(s, OHCI_INTR_RHSC); + } +} + static void ohci_async_cancel_device(OHCIState *ohci, USBDevice *dev) { if (ohci->async_td && @@ -1799,11 +1735,72 @@ static void ohci_async_cancel_device(OHCIState *ohci, USBDevice *dev) } } -static const MemoryRegionOps ohci_mem_ops = { - .read = ohci_mem_read, - .write = ohci_mem_write, - .endianness = DEVICE_LITTLE_ENDIAN, -}; +static void ohci_child_detach(USBPort *port1, USBDevice *child) +{ + OHCIState *s = port1->opaque; + + ohci_async_cancel_device(s, child); +} + +static void ohci_detach(USBPort *port1) +{ + OHCIState *s = port1->opaque; + OHCIPort *port = &s->rhport[port1->index]; + uint32_t old_state = port->ctrl; + + ohci_async_cancel_device(s, port1->dev); + + /* set connect status */ + if (port->ctrl & OHCI_PORT_CCS) { + port->ctrl &= ~OHCI_PORT_CCS; + port->ctrl |= OHCI_PORT_CSC; + } + /* disable port */ + if (port->ctrl & OHCI_PORT_PES) { + port->ctrl &= ~OHCI_PORT_PES; + port->ctrl |= OHCI_PORT_PESC; + } + trace_usb_ohci_port_detach(port1->index); + + if (old_state != port->ctrl) { + ohci_set_interrupt(s, OHCI_INTR_RHSC); + } +} + +static void ohci_wakeup(USBPort *port1) +{ + OHCIState *s = port1->opaque; + OHCIPort *port = &s->rhport[port1->index]; + uint32_t intr = 0; + if (port->ctrl & OHCI_PORT_PSS) { + trace_usb_ohci_port_wakeup(port1->index); + port->ctrl |= OHCI_PORT_PSSC; + port->ctrl &= ~OHCI_PORT_PSS; + intr = OHCI_INTR_RHSC; + } + /* Note that the controller can be suspended even if this port is not */ + if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) { + trace_usb_ohci_remote_wakeup(s->name); + /* This is the one state transition the controller can do by itself */ + s->ctl &= ~OHCI_CTL_HCFS; + s->ctl |= OHCI_USB_RESUME; + /* + * In suspend mode only ResumeDetected is possible, not RHSC: + * see the OHCI spec 5.1.2.3. + */ + intr = OHCI_INTR_RD; + } + ohci_set_interrupt(s, intr); +} + +static void ohci_async_complete_packet(USBPort *port, USBPacket *packet) +{ + OHCIState *ohci = container_of(packet, OHCIState, usb_packet); + + trace_usb_ohci_async_complete(); + ohci->async_complete = true; + ohci_process_lists(ohci, 1); +} static USBPortOps ohci_port_ops = { .attach = ohci_attach, From b6b0c066f5750c3c977b647509f225ba06038b60 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Tue, 25 Jan 2022 14:33:20 +0100 Subject: [PATCH 06/35] usb/ohci: Merge ohci_async_cancel_device() into ohci_child_detach() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These two do the same and only used once so no need to have two functions, simplify by merging them. Signed-off-by: BALATON Zoltan Reviewed-by: Philippe Mathieu-Daudé Message-Id: <5fc8ba0bbf55703014d22dd06ab2f9eabaf370bf.1643117600.git.balaton@eik.bme.hu> Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 190f5a8aba..09d07367cc 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -1725,8 +1725,10 @@ static void ohci_attach(USBPort *port1) } } -static void ohci_async_cancel_device(OHCIState *ohci, USBDevice *dev) +static void ohci_child_detach(USBPort *port1, USBDevice *dev) { + OHCIState *ohci = port1->opaque; + if (ohci->async_td && usb_packet_is_inflight(&ohci->usb_packet) && ohci->usb_packet.ep->dev == dev) { @@ -1735,20 +1737,13 @@ static void ohci_async_cancel_device(OHCIState *ohci, USBDevice *dev) } } -static void ohci_child_detach(USBPort *port1, USBDevice *child) -{ - OHCIState *s = port1->opaque; - - ohci_async_cancel_device(s, child); -} - static void ohci_detach(USBPort *port1) { OHCIState *s = port1->opaque; OHCIPort *port = &s->rhport[port1->index]; uint32_t old_state = port->ctrl; - ohci_async_cancel_device(s, port1->dev); + ohci_child_detach(port1, port1->dev); /* set connect status */ if (port->ctrl & OHCI_PORT_CCS) { From 3a4d06f26f2606dd5029b05b4aff97f198455249 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Tue, 25 Jan 2022 14:33:20 +0100 Subject: [PATCH 07/35] usb/ohci: Don't use packet from OHCIState for isochronous transfers Since isochronous transfers cannot be handled async (the function returns error in that case) we don't need to remember the packet. Avoid using the usb_packet field in OHCIState (as that can be a waiting async packet on another endpoint) and allocate and use a local USBPacket for the iso transfer instead. After this we don't have to care if we're called from a completion callback or not so we can drop that parameter as well. Signed-off-by: BALATON Zoltan Message-Id: Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 71 +++++++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 09d07367cc..895b29fb86 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -548,8 +548,7 @@ static int ohci_copy_iso_td(OHCIState *ohci, #define USUB(a, b) ((int16_t)((uint16_t)(a) - (uint16_t)(b))) -static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, - int completion) +static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed) { int dir; size_t len = 0; @@ -559,6 +558,9 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, int i; USBDevice *dev; USBEndpoint *ep; + USBPacket *pkt; + uint8_t buf[8192]; + bool int_req; struct ohci_iso_td iso_td; uint32_t addr; uint16_t starting_frame; @@ -693,40 +695,42 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, } else { len = end_addr - start_addr + 1; } - if (len > sizeof(ohci->usb_buf)) { - len = sizeof(ohci->usb_buf); + if (len > sizeof(buf)) { + len = sizeof(buf); } if (len && dir != OHCI_TD_DIR_IN) { - if (ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, len, + if (ohci_copy_iso_td(ohci, start_addr, end_addr, buf, len, DMA_DIRECTION_TO_DEVICE)) { ohci_die(ohci); return 1; } } - if (!completion) { - bool int_req = relative_frame_number == frame_count && - OHCI_BM(iso_td.flags, TD_DI) == 0; - dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA)); - if (dev == NULL) { - trace_usb_ohci_td_dev_error(); - return 1; - } - ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN)); - usb_packet_setup(&ohci->usb_packet, pid, ep, 0, addr, false, int_req); - usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, len); - usb_handle_packet(dev, &ohci->usb_packet); - if (ohci->usb_packet.status == USB_RET_ASYNC) { - usb_device_flush_ep_queue(dev, ep); - return 1; - } + dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA)); + if (dev == NULL) { + trace_usb_ohci_td_dev_error(); + return 1; } - if (ohci->usb_packet.status == USB_RET_SUCCESS) { - ret = ohci->usb_packet.actual_length; + ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN)); + pkt = g_new0(USBPacket, 1); + usb_packet_init(pkt); + int_req = relative_frame_number == frame_count && + OHCI_BM(iso_td.flags, TD_DI) == 0; + usb_packet_setup(pkt, pid, ep, 0, addr, false, int_req); + usb_packet_addbuf(pkt, buf, len); + usb_handle_packet(dev, pkt); + if (pkt->status == USB_RET_ASYNC) { + usb_device_flush_ep_queue(dev, ep); + g_free(pkt); + return 1; + } + if (pkt->status == USB_RET_SUCCESS) { + ret = pkt->actual_length; } else { - ret = ohci->usb_packet.status; + ret = pkt->status; } + g_free(pkt); trace_usb_ohci_iso_td_so(start_offset, end_offset, start_addr, end_addr, str, len, ret); @@ -734,7 +738,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, /* Writeback */ if (dir == OHCI_TD_DIR_IN && ret >= 0 && ret <= len) { /* IN transfer succeeded */ - if (ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, ret, + if (ohci_copy_iso_td(ohci, start_addr, end_addr, buf, ret, DMA_DIRECTION_FROM_DEVICE)) { ohci_die(ohci); return 1; @@ -1057,7 +1061,7 @@ exit_no_retire: } /* Service an endpoint list. Returns nonzero if active TD were found. */ -static int ohci_service_ed_list(OHCIState *ohci, uint32_t head, int completion) +static int ohci_service_ed_list(OHCIState *ohci, uint32_t head) { struct ohci_ed ed; uint32_t next_ed; @@ -1108,8 +1112,9 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head, int completion) break; } else { /* Handle isochronous endpoints */ - if (ohci_service_iso_td(ohci, &ed, completion)) + if (ohci_service_iso_td(ohci, &ed)) { break; + } } } @@ -1136,20 +1141,20 @@ static void ohci_sof(OHCIState *ohci) } /* Process Control and Bulk lists. */ -static void ohci_process_lists(OHCIState *ohci, int completion) +static void ohci_process_lists(OHCIState *ohci) { if ((ohci->ctl & OHCI_CTL_CLE) && (ohci->status & OHCI_STATUS_CLF)) { if (ohci->ctrl_cur && ohci->ctrl_cur != ohci->ctrl_head) { trace_usb_ohci_process_lists(ohci->ctrl_head, ohci->ctrl_cur); } - if (!ohci_service_ed_list(ohci, ohci->ctrl_head, completion)) { + if (!ohci_service_ed_list(ohci, ohci->ctrl_head)) { ohci->ctrl_cur = 0; ohci->status &= ~OHCI_STATUS_CLF; } } if ((ohci->ctl & OHCI_CTL_BLE) && (ohci->status & OHCI_STATUS_BLF)) { - if (!ohci_service_ed_list(ohci, ohci->bulk_head, completion)) { + if (!ohci_service_ed_list(ohci, ohci->bulk_head)) { ohci->bulk_cur = 0; ohci->status &= ~OHCI_STATUS_BLF; } @@ -1173,7 +1178,7 @@ static void ohci_frame_boundary(void *opaque) int n; n = ohci->frame_number & 0x1f; - ohci_service_ed_list(ohci, le32_to_cpu(hcca.intr[n]), 0); + ohci_service_ed_list(ohci, le32_to_cpu(hcca.intr[n])); } /* Cancel all pending packets if either of the lists has been disabled. */ @@ -1181,7 +1186,7 @@ static void ohci_frame_boundary(void *opaque) ohci_stop_endpoints(ohci); } ohci->old_ctl = ohci->ctl; - ohci_process_lists(ohci, 0); + ohci_process_lists(ohci); /* Stop if UnrecoverableError happened or ohci_sof will crash */ if (ohci->intr_status & OHCI_INTR_UE) { @@ -1794,7 +1799,7 @@ static void ohci_async_complete_packet(USBPort *port, USBPacket *packet) trace_usb_ohci_async_complete(); ohci->async_complete = true; - ohci_process_lists(ohci, 1); + ohci_process_lists(ohci); } static USBPortOps ohci_port_ops = { From 18404ff111e43d218b36b69ae002cae58acef352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:12:57 +0100 Subject: [PATCH 08/35] audio: replace open-coded buffer arithmetic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace open-coded buffer arithmetic with the new function audio_ring_posb(). That's the position in backward direction of a given point at a given distance. Signed-off-by: Volker Rümelin Reviewed-by: Akihiko Odaki Message-Id: <20220301191311.26695-1-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/audio.c | 25 +++++++------------------ audio/audio_int.h | 13 +++++++++++++ audio/coreaudio.c | 10 ++++------ audio/sdlaudio.c | 11 +++++------ 4 files changed, 29 insertions(+), 30 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index dc28685d22..e7a139e289 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -574,19 +574,13 @@ static size_t audio_pcm_sw_get_rpos_in(SWVoiceIn *sw) { HWVoiceIn *hw = sw->hw; ssize_t live = hw->total_samples_captured - sw->total_hw_samples_acquired; - ssize_t rpos; if (audio_bug(__func__, live < 0 || live > hw->conv_buf->size)) { dolog("live=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size); return 0; } - rpos = hw->conv_buf->pos - live; - if (rpos >= 0) { - return rpos; - } else { - return hw->conv_buf->size + rpos; - } + return audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size); } static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size) @@ -1394,12 +1388,10 @@ void audio_generic_run_buffer_in(HWVoiceIn *hw) void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size) { - ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul; + size_t start; - if (start < 0) { - start += hw->size_emul; - } - assert(start >= 0 && start < hw->size_emul); + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); + assert(start < hw->size_emul); *size = MIN(*size, hw->pending_emul); *size = MIN(*size, hw->size_emul - start); @@ -1415,13 +1407,10 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size) void audio_generic_run_buffer_out(HWVoiceOut *hw) { while (hw->pending_emul) { - size_t write_len, written; - ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul; + size_t write_len, written, start; - if (start < 0) { - start += hw->size_emul; - } - assert(start >= 0 && start < hw->size_emul); + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); + assert(start < hw->size_emul); write_len = MIN(hw->pending_emul, hw->size_emul - start); diff --git a/audio/audio_int.h b/audio/audio_int.h index 428a091d05..71be162271 100644 --- a/audio/audio_int.h +++ b/audio/audio_int.h @@ -266,6 +266,19 @@ static inline size_t audio_ring_dist(size_t dst, size_t src, size_t len) return (dst >= src) ? (dst - src) : (len - src + dst); } +/** + * audio_ring_posb() - returns new position in ringbuffer in backward + * direction at given distance + * + * @pos: current position in ringbuffer + * @dist: distance in ringbuffer to walk in reverse direction + * @len: size of ringbuffer + */ +static inline size_t audio_ring_posb(size_t pos, size_t dist, size_t len) +{ + return pos >= dist ? pos - dist : len - dist + pos; +} + #define dolog(fmt, ...) AUD_log(AUDIO_CAP, fmt, ## __VA_ARGS__) #ifdef DEBUG diff --git a/audio/coreaudio.c b/audio/coreaudio.c index d8a21d3e50..1fdd1d4b14 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -333,12 +333,10 @@ static OSStatus audioDeviceIOProc( len = frameCount * hw->info.bytes_per_frame; while (len) { - size_t write_len; - ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul; - if (start < 0) { - start += hw->size_emul; - } - assert(start >= 0 && start < hw->size_emul); + size_t write_len, start; + + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); + assert(start < hw->size_emul); write_len = MIN(MIN(hw->pending_emul, len), hw->size_emul - start); diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c index c68c62a3e4..d6f3aa1a9a 100644 --- a/audio/sdlaudio.c +++ b/audio/sdlaudio.c @@ -224,12 +224,11 @@ static void sdl_callback_out(void *opaque, Uint8 *buf, int len) /* dolog("callback_out: len=%d avail=%zu\n", len, hw->pending_emul); */ while (hw->pending_emul && len) { - size_t write_len; - ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul; - if (start < 0) { - start += hw->size_emul; - } - assert(start >= 0 && start < hw->size_emul); + size_t write_len, start; + + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, + hw->size_emul); + assert(start < hw->size_emul); write_len = MIN(MIN(hw->pending_emul, len), hw->size_emul - start); From 8e56a172a15708c6b7382fbf3441bed26bc0ffef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:12:58 +0100 Subject: [PATCH 09/35] audio: move function audio_pcm_hw_clip_out() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the function audio_pcm_hw_clip_out() into the correct section 'Hard voice (playback)'. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-2-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/audio.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index e7a139e289..dfd32912da 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -548,25 +548,6 @@ static size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw) return live; } -static void audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf, size_t len) -{ - size_t clipped = 0; - size_t pos = hw->mix_buf->pos; - - while (len) { - st_sample *src = hw->mix_buf->samples + pos; - uint8_t *dst = advance(pcm_buf, clipped * hw->info.bytes_per_frame); - size_t samples_till_end_of_buf = hw->mix_buf->size - pos; - size_t samples_to_clip = MIN(len, samples_till_end_of_buf); - - hw->clip(dst, src, samples_to_clip); - - pos = (pos + samples_to_clip) % hw->mix_buf->size; - len -= samples_to_clip; - clipped += samples_to_clip; - } -} - /* * Soft voice (capture) */ @@ -677,6 +658,25 @@ static size_t audio_pcm_hw_get_live_out (HWVoiceOut *hw, int *nb_live) return 0; } +static void audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf, size_t len) +{ + size_t clipped = 0; + size_t pos = hw->mix_buf->pos; + + while (len) { + st_sample *src = hw->mix_buf->samples + pos; + uint8_t *dst = advance(pcm_buf, clipped * hw->info.bytes_per_frame); + size_t samples_till_end_of_buf = hw->mix_buf->size - pos; + size_t samples_to_clip = MIN(len, samples_till_end_of_buf); + + hw->clip(dst, src, samples_to_clip); + + pos = (pos + samples_to_clip) % hw->mix_buf->size; + len -= samples_to_clip; + clipped += samples_to_clip; + } +} + /* * Soft voice (playback) */ From 251f15496ec5f274f100cd155eb60b9add96196e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:12:59 +0100 Subject: [PATCH 10/35] audio: add function audio_pcm_hw_conv_in() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a function audio_pcm_hw_conv_in() similar to the existing counterpart function audio_pcm_hw_clip_out(). This function reduces the number of calls to the pcm_ops functions get_buffer_in() and put_buffer_in(). That's one less call to get_buffer_in() and put_buffer_in() every time the conv_buffer wraps around. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-3-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/audio.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index dfd32912da..f28e91853f 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -548,6 +548,24 @@ static size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw) return live; } +static size_t audio_pcm_hw_conv_in(HWVoiceIn *hw, void *pcm_buf, size_t samples) +{ + size_t conv = 0; + STSampleBuffer *conv_buf = hw->conv_buf; + + while (samples) { + uint8_t *src = advance(pcm_buf, conv * hw->info.bytes_per_frame); + size_t proc = MIN(samples, conv_buf->size - conv_buf->pos); + + hw->conv(conv_buf->samples + conv_buf->pos, src, proc); + conv_buf->pos = (conv_buf->pos + proc) % conv_buf->size; + samples -= proc; + conv += proc; + } + + return conv; +} + /* * Soft voice (capture) */ @@ -1219,7 +1237,6 @@ static void audio_run_out (AudioState *s) static size_t audio_pcm_hw_run_in(HWVoiceIn *hw, size_t samples) { size_t conv = 0; - STSampleBuffer *conv_buf = hw->conv_buf; if (hw->pcm_ops->run_buffer_in) { hw->pcm_ops->run_buffer_in(hw); @@ -1235,11 +1252,7 @@ static size_t audio_pcm_hw_run_in(HWVoiceIn *hw, size_t samples) break; } - proc = MIN(size / hw->info.bytes_per_frame, - conv_buf->size - conv_buf->pos); - - hw->conv(conv_buf->samples + conv_buf->pos, buf, proc); - conv_buf->pos = (conv_buf->pos + proc) % conv_buf->size; + proc = audio_pcm_hw_conv_in(hw, buf, size / hw->info.bytes_per_frame); samples -= proc; conv += proc; From 0ceb26af0c2f4f07ed5d2c2493552501926df45b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:00 +0100 Subject: [PATCH 11/35] audio: inline function audio_pcm_sw_get_rpos_in() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify code by inlining function audio_pcm_sw_get_rpos_in() at the only call site and remove the duplicated audio_bug() test. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-4-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/audio.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index f28e91853f..35437986d9 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -569,37 +569,24 @@ static size_t audio_pcm_hw_conv_in(HWVoiceIn *hw, void *pcm_buf, size_t samples) /* * Soft voice (capture) */ -static size_t audio_pcm_sw_get_rpos_in(SWVoiceIn *sw) -{ - HWVoiceIn *hw = sw->hw; - ssize_t live = hw->total_samples_captured - sw->total_hw_samples_acquired; - - if (audio_bug(__func__, live < 0 || live > hw->conv_buf->size)) { - dolog("live=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size); - return 0; - } - - return audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size); -} - static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size) { HWVoiceIn *hw = sw->hw; size_t samples, live, ret = 0, swlim, isamp, osamp, rpos, total = 0; struct st_sample *src, *dst = sw->buf; - rpos = audio_pcm_sw_get_rpos_in(sw) % hw->conv_buf->size; - live = hw->total_samples_captured - sw->total_hw_samples_acquired; + if (!live) { + return 0; + } if (audio_bug(__func__, live > hw->conv_buf->size)) { dolog("live_in=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size); return 0; } + rpos = audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size); + samples = size / sw->info.bytes_per_frame; - if (!live) { - return 0; - } swlim = (live * sw->ratio) >> 32; swlim = MIN (swlim, samples); From 30ff5e24a31e818f2ae41861727866a35bfdbe23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:01 +0100 Subject: [PATCH 12/35] paaudio: increase default latency to 46ms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a patch to improve the pulseaudio playback experience. Asking pulseaudio for a playback latency of 15ms is quite demanding. Increase this to 46ms. The total playback latency now is 31ms larger. One of the next patches will reduce the total playback latency again by more than 46ms. Here is a quote from the PulseAudio Latency Control documentation: 'For the sake of (...) drop-out safety always make sure to pick the highest latency possible that fulfills your needs.' Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-5-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/paaudio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audio/paaudio.c b/audio/paaudio.c index 75401d5391..9df1e69c08 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -744,7 +744,7 @@ static int qpa_validate_per_direction_opts(Audiodev *dev, { if (!pdo->has_latency) { pdo->has_latency = true; - pdo->latency = 15000; + pdo->latency = 46440; } return 1; } From 369829a435ad87d2ac8ae79e6dc61e8c76d6890b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:02 +0100 Subject: [PATCH 13/35] jackaudio: use more jack audio buffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The next patch reduces the effective qemu playback buffer size by timer-period. Increase the number of jack audio buffers by one to preserve the total effective buffer size. The size of one jack audio buffer is 512 samples. With audio defaults that's 512 samples / 44100 samples/s = 11.6 ms and only slightly larger than the timer-period of 10 ms. The larger jack audio buffer increases audio dropout safety, because the high priority jack-audio worker threads can provide audio data for a longer period of time as with a smaller buffer and more audio data in the mixing engine buffer that they can't access. Signed-off-by: Volker Rümelin Reviewed-by: Christian Schoenebeck Message-Id: <20220301191311.26695-6-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/jackaudio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/audio/jackaudio.c b/audio/jackaudio.c index 317009e936..26246c3a8b 100644 --- a/audio/jackaudio.c +++ b/audio/jackaudio.c @@ -483,8 +483,8 @@ static int qjack_client_init(QJackClient *c) c->buffersize = 512; } - /* create a 2 period buffer */ - qjack_buffer_create(&c->fifo, c->nchannels, c->buffersize * 2); + /* create a 3 period buffer */ + qjack_buffer_create(&c->fifo, c->nchannels, c->buffersize * 3); qjack_client_connect_ports(c); c->state = QJACK_STATE_RUNNING; From a806f95904cdb825c7b9d0cda95c8944907fb226 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:03 +0100 Subject: [PATCH 14/35] audio: copy playback stream in sequential order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the code to copy the playback stream in sequential order. The advantage can be seen in the next patches where the stream copy operation effectively becomes a write through operation. The following diagram shows the average buffer fill level and the stream copy sequence. ### represents a timer_period sized chunk. The rest of the buffer sizes are not to scale. With current code: |--------| |#####111| |---#####| sw->buf mix_buf backend buffer 1. clip |--------| |---#####| |111##222| sw->buf mix_buf backend buffer 2. write to audio device 333 -> |--------| |---#####| |---111##| -> 222 sw->buf mix_buf backend buffer 3a. sw device write |-----333| |---#####| |---111##| sw->buf mix_buf backend buffer 3b. resample and mix |--------| |333#####| |---111##| sw->buf mix_buf backend buffer With this patch: 111 -> |--------| |---#####| |---#####| sw->buf mix_buf backend buffer 1a: sw device write |-----111| |---#####| |---#####| sw->buf mix_buf backend buffer 1b. resample and mix |--------| |111##222| |---#####| sw->buf mix_buf backend buffer 2. clip |--------| |---111##| |222##333| sw->buf mix_buf backend buffer 3. write to audio device |--------| |---111##| |---222##| -> 333 sw->buf mix_buf backend buffer The effective total playback buffer size is reduced by timer_period. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-7-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/audio.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 35437986d9..9e2d7fb209 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -1134,6 +1134,15 @@ static void audio_run_out (AudioState *s) size_t played, live, prev_rpos, free; int nb_live; + for (sw = hw->sw_head.lh_first; sw; sw = sw->entries.le_next) { + if (sw->active) { + free = audio_get_free(sw); + if (free > 0) { + sw->callback.fn(sw->callback.opaque, free); + } + } + } + live = audio_pcm_hw_get_live_out (hw, &nb_live); if (!nb_live) { live = 0; @@ -1162,14 +1171,6 @@ static void audio_run_out (AudioState *s) } if (!live) { - for (sw = hw->sw_head.lh_first; sw; sw = sw->entries.le_next) { - if (sw->active) { - free = audio_get_free (sw); - if (free > 0) { - sw->callback.fn (sw->callback.opaque, free); - } - } - } if (hw->pcm_ops->run_buffer_out) { hw->pcm_ops->run_buffer_out(hw); } @@ -1210,13 +1211,6 @@ static void audio_run_out (AudioState *s) if (!sw->total_hw_samples_mixed) { sw->empty = 1; } - - if (sw->active) { - free = audio_get_free (sw); - if (free > 0) { - sw->callback.fn (sw->callback.opaque, free); - } - } } } } From 33940dd3368f1c8b490f9aa3a762727f6b6df106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:04 +0100 Subject: [PATCH 15/35] audio: add pcm_ops function table for capture backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a pcm_ops function table for the capture backend. This avoids additional code in the next patches to test if the pcm_ops table is available. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-8-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/audio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/audio/audio.c b/audio/audio.c index 9e2d7fb209..55f885f8e9 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -1804,6 +1804,7 @@ void AUD_remove_card (QEMUSoundCard *card) g_free (card->name); } +static struct audio_pcm_ops capture_pcm_ops; CaptureVoiceOut *AUD_add_capture( AudioState *s, @@ -1849,6 +1850,7 @@ CaptureVoiceOut *AUD_add_capture( hw = &cap->hw; hw->s = s; + hw->pcm_ops = &capture_pcm_ops; QLIST_INIT (&hw->sw_head); QLIST_INIT (&cap->cb_head); From 669b95229d13e3c521c2f50bcc9ca0503efb3c5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:05 +0100 Subject: [PATCH 16/35] Revert "audio: fix wavcapture segfault" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit cbaf25d1f59ee13fc7542a06ea70784f2e000c04. Since previous commit every audio backend has a pcm_ops function table. It's no longer necessary to test if the table is available. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-9-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/audio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 55f885f8e9..c420a8bd1c 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -612,7 +612,7 @@ static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size) total += isamp; } - if (hw->pcm_ops && !hw->pcm_ops->volume_in) { + if (!hw->pcm_ops->volume_in) { mixeng_volume (sw->buf, ret, &sw->vol); } @@ -718,7 +718,7 @@ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t size) if (swlim) { sw->conv (sw->buf, buf, swlim); - if (sw->hw->pcm_ops && !sw->hw->pcm_ops->volume_out) { + if (!sw->hw->pcm_ops->volume_out) { mixeng_volume (sw->buf, swlim, &sw->vol); } } From 9833438ef624155de879d4ed57ecfcd3464a0bbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:06 +0100 Subject: [PATCH 17/35] audio: restore mixing-engine playback buffer size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit ff095e5231 "audio: api for mixeng code free backends" introduced another FIFO for the audio subsystem with exactly the same size as the mixing-engine FIFO. Most audio backends use this generic FIFO. The generic FIFO used together with the mixing-engine FIFO doubles the audio FIFO size, because that's just two independent FIFOs connected together in series. For audio playback this nearly doubles the playback latency. This patch restores the effective mixing-engine playback buffer size to a pre v4.2.0 size by only accepting the amount of samples for the mixing-engine queue which the downstream queue accepts. Signed-off-by: Volker Rümelin Reviewed-by: Akihiko Odaki Message-Id: <20220301191311.26695-10-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/alsaaudio.c | 1 + audio/audio.c | 69 +++++++++++++++++++++++++++++++++++------------ audio/audio_int.h | 7 ++++- audio/coreaudio.c | 3 +++ audio/jackaudio.c | 1 + audio/noaudio.c | 1 + audio/ossaudio.c | 12 +++++++++ audio/sdlaudio.c | 3 +++ audio/wavaudio.c | 1 + 9 files changed, 80 insertions(+), 18 deletions(-) diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c index 2b9789e647..b04716a6cc 100644 --- a/audio/alsaaudio.c +++ b/audio/alsaaudio.c @@ -916,6 +916,7 @@ static struct audio_pcm_ops alsa_pcm_ops = { .init_out = alsa_init_out, .fini_out = alsa_fini_out, .write = alsa_write, + .buffer_get_free = audio_generic_buffer_get_free, .run_buffer_out = audio_generic_run_buffer_out, .enable_out = alsa_enable_out, diff --git a/audio/audio.c b/audio/audio.c index c420a8bd1c..a88572e713 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -663,6 +663,12 @@ static size_t audio_pcm_hw_get_live_out (HWVoiceOut *hw, int *nb_live) return 0; } +static size_t audio_pcm_hw_get_free(HWVoiceOut *hw) +{ + return (hw->pcm_ops->buffer_get_free ? hw->pcm_ops->buffer_get_free(hw) : + INT_MAX) / hw->info.bytes_per_frame; +} + static void audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf, size_t len) { size_t clipped = 0; @@ -687,7 +693,8 @@ static void audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf, size_t len) */ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t size) { - size_t hwsamples, samples, isamp, osamp, wpos, live, dead, left, swlim, blck; + size_t hwsamples, samples, isamp, osamp, wpos, live, dead, left, blck; + size_t hw_free; size_t ret = 0, pos = 0, total = 0; if (!sw) { @@ -710,27 +717,28 @@ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t size) } wpos = (sw->hw->mix_buf->pos + live) % hwsamples; - samples = size / sw->info.bytes_per_frame; dead = hwsamples - live; - swlim = ((int64_t) dead << 32) / sw->ratio; - swlim = MIN (swlim, samples); - if (swlim) { - sw->conv (sw->buf, buf, swlim); + hw_free = audio_pcm_hw_get_free(sw->hw); + hw_free = hw_free > live ? hw_free - live : 0; + samples = ((int64_t)MIN(dead, hw_free) << 32) / sw->ratio; + samples = MIN(samples, size / sw->info.bytes_per_frame); + if (samples) { + sw->conv(sw->buf, buf, samples); if (!sw->hw->pcm_ops->volume_out) { - mixeng_volume (sw->buf, swlim, &sw->vol); + mixeng_volume(sw->buf, samples, &sw->vol); } } - while (swlim) { + while (samples) { dead = hwsamples - live; left = hwsamples - wpos; blck = MIN (dead, left); if (!blck) { break; } - isamp = swlim; + isamp = samples; osamp = blck; st_rate_flow_mix ( sw->rate, @@ -740,7 +748,7 @@ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t size) &osamp ); ret += isamp; - swlim -= isamp; + samples -= isamp; pos += isamp; live += osamp; wpos = (wpos + osamp) % hwsamples; @@ -1002,6 +1010,11 @@ static size_t audio_get_avail (SWVoiceIn *sw) return (((int64_t) live << 32) / sw->ratio) * sw->info.bytes_per_frame; } +static size_t audio_sw_bytes_free(SWVoiceOut *sw, size_t free) +{ + return (((int64_t)free << 32) / sw->ratio) * sw->info.bytes_per_frame; +} + static size_t audio_get_free(SWVoiceOut *sw) { size_t live, dead; @@ -1021,13 +1034,11 @@ static size_t audio_get_free(SWVoiceOut *sw) dead = sw->hw->mix_buf->size - live; #ifdef DEBUG_OUT - dolog ("%s: get_free live %zu dead %zu ret %" PRId64 "\n", - SW_NAME (sw), - live, dead, (((int64_t) dead << 32) / sw->ratio) * - sw->info.bytes_per_frame); + dolog("%s: get_free live %zu dead %zu sw_bytes %zu\n", + SW_NAME(sw), live, dead, audio_sw_bytes_free(sw, dead)); #endif - return (((int64_t) dead << 32) / sw->ratio) * sw->info.bytes_per_frame; + return dead; } static void audio_capture_mix_and_clear(HWVoiceOut *hw, size_t rpos, @@ -1131,12 +1142,21 @@ static void audio_run_out (AudioState *s) } while ((hw = audio_pcm_hw_find_any_enabled_out(s, hw))) { - size_t played, live, prev_rpos, free; + size_t played, live, prev_rpos; + size_t hw_free = audio_pcm_hw_get_free(hw); int nb_live; for (sw = hw->sw_head.lh_first; sw; sw = sw->entries.le_next) { if (sw->active) { - free = audio_get_free(sw); + size_t sw_free = audio_get_free(sw); + size_t free; + + if (hw_free > sw->total_hw_samples_mixed) { + free = audio_sw_bytes_free(sw, + MIN(sw_free, hw_free - sw->total_hw_samples_mixed)); + } else { + free = 0; + } if (free > 0) { sw->callback.fn(sw->callback.opaque, free); } @@ -1398,6 +1418,15 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size) hw->pending_emul -= size; } +size_t audio_generic_buffer_get_free(HWVoiceOut *hw) +{ + if (hw->buf_emul) { + return hw->size_emul - hw->pending_emul; + } else { + return hw->samples * hw->info.bytes_per_frame; + } +} + void audio_generic_run_buffer_out(HWVoiceOut *hw) { while (hw->pending_emul) { @@ -1445,6 +1474,12 @@ size_t audio_generic_write(HWVoiceOut *hw, void *buf, size_t size) { size_t total = 0; + if (hw->pcm_ops->buffer_get_free) { + size_t free = hw->pcm_ops->buffer_get_free(hw); + + size = MIN(size, free); + } + while (total < size) { size_t dst_size = size - total; size_t copy_size, proc; diff --git a/audio/audio_int.h b/audio/audio_int.h index 71be162271..2a6914d2aa 100644 --- a/audio/audio_int.h +++ b/audio/audio_int.h @@ -161,10 +161,14 @@ struct audio_pcm_ops { void (*fini_out)(HWVoiceOut *hw); size_t (*write) (HWVoiceOut *hw, void *buf, size_t size); void (*run_buffer_out)(HWVoiceOut *hw); + /* + * Get the free output buffer size. This is an upper limit. The size + * returned by function get_buffer_out may be smaller. + */ + size_t (*buffer_get_free)(HWVoiceOut *hw); /* * get a buffer that after later can be passed to put_buffer_out; optional * returns the buffer, and writes it's size to size (in bytes) - * this is unrelated to the above buffer_size_out function */ void *(*get_buffer_out)(HWVoiceOut *hw, size_t *size); /* @@ -190,6 +194,7 @@ void audio_generic_run_buffer_in(HWVoiceIn *hw); void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size); void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size); void audio_generic_run_buffer_out(HWVoiceOut *hw); +size_t audio_generic_buffer_get_free(HWVoiceOut *hw); void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size); size_t audio_generic_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size); size_t audio_generic_write(HWVoiceOut *hw, void *buf, size_t size); diff --git a/audio/coreaudio.c b/audio/coreaudio.c index 1fdd1d4b14..91ea6ae975 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -283,6 +283,7 @@ static int coreaudio_buf_unlock (coreaudioVoiceOut *core, const char *fn_name) coreaudio_buf_unlock(core, "coreaudio_" #name); \ return ret; \ } +COREAUDIO_WRAPPER_FUNC(buffer_get_free, size_t, (HWVoiceOut *hw), (hw)) COREAUDIO_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size), (hw, size)) COREAUDIO_WRAPPER_FUNC(put_buffer_out, size_t, @@ -652,6 +653,8 @@ static struct audio_pcm_ops coreaudio_pcm_ops = { .fini_out = coreaudio_fini_out, /* wrapper for audio_generic_write */ .write = coreaudio_write, + /* wrapper for audio_generic_buffer_get_free */ + .buffer_get_free = coreaudio_buffer_get_free, /* wrapper for audio_generic_get_buffer_out */ .get_buffer_out = coreaudio_get_buffer_out, /* wrapper for audio_generic_put_buffer_out */ diff --git a/audio/jackaudio.c b/audio/jackaudio.c index 26246c3a8b..bf757250b5 100644 --- a/audio/jackaudio.c +++ b/audio/jackaudio.c @@ -652,6 +652,7 @@ static struct audio_pcm_ops jack_pcm_ops = { .init_out = qjack_init_out, .fini_out = qjack_fini_out, .write = qjack_write, + .buffer_get_free = audio_generic_buffer_get_free, .run_buffer_out = audio_generic_run_buffer_out, .enable_out = qjack_enable_out, diff --git a/audio/noaudio.c b/audio/noaudio.c index aac87dbc93..84a6bfbb1c 100644 --- a/audio/noaudio.c +++ b/audio/noaudio.c @@ -118,6 +118,7 @@ static struct audio_pcm_ops no_pcm_ops = { .init_out = no_init_out, .fini_out = no_fini_out, .write = no_write, + .buffer_get_free = audio_generic_buffer_get_free, .run_buffer_out = audio_generic_run_buffer_out, .enable_out = no_enable_out, diff --git a/audio/ossaudio.c b/audio/ossaudio.c index 60eff66424..1bd6800840 100644 --- a/audio/ossaudio.c +++ b/audio/ossaudio.c @@ -389,6 +389,17 @@ static void oss_run_buffer_out(HWVoiceOut *hw) } } +static size_t oss_buffer_get_free(HWVoiceOut *hw) +{ + OSSVoiceOut *oss = (OSSVoiceOut *)hw; + + if (oss->mmapped) { + return INT_MAX; + } else { + return audio_generic_buffer_get_free(hw); + } +} + static void *oss_get_buffer_out(HWVoiceOut *hw, size_t *size) { OSSVoiceOut *oss = (OSSVoiceOut *) hw; @@ -750,6 +761,7 @@ static struct audio_pcm_ops oss_pcm_ops = { .init_out = oss_init_out, .fini_out = oss_fini_out, .write = oss_write, + .buffer_get_free = oss_buffer_get_free, .run_buffer_out = oss_run_buffer_out, .get_buffer_out = oss_get_buffer_out, .put_buffer_out = oss_put_buffer_out, diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c index d6f3aa1a9a..e605c787ba 100644 --- a/audio/sdlaudio.c +++ b/audio/sdlaudio.c @@ -309,6 +309,7 @@ static void sdl_callback_in(void *opaque, Uint8 *buf, int len) SDL_UnlockAudioDevice(sdl->devid); \ } +SDL_WRAPPER_FUNC(buffer_get_free, size_t, (HWVoiceOut *hw), (hw), Out) SDL_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size), (hw, size), Out) SDL_WRAPPER_FUNC(put_buffer_out, size_t, @@ -471,6 +472,8 @@ static struct audio_pcm_ops sdl_pcm_ops = { .fini_out = sdl_fini_out, /* wrapper for audio_generic_write */ .write = sdl_write, + /* wrapper for audio_generic_buffer_get_free */ + .buffer_get_free = sdl_buffer_get_free, /* wrapper for audio_generic_get_buffer_out */ .get_buffer_out = sdl_get_buffer_out, /* wrapper for audio_generic_put_buffer_out */ diff --git a/audio/wavaudio.c b/audio/wavaudio.c index 20e6853f85..ac666335c7 100644 --- a/audio/wavaudio.c +++ b/audio/wavaudio.c @@ -197,6 +197,7 @@ static struct audio_pcm_ops wav_pcm_ops = { .init_out = wav_init_out, .fini_out = wav_fini_out, .write = wav_write_out, + .buffer_get_free = audio_generic_buffer_get_free, .run_buffer_out = audio_generic_run_buffer_out, .enable_out = wav_enable_out, }; From ddf2050ce67617b3063a48215694a932d0af71ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:07 +0100 Subject: [PATCH 18/35] paaudio: reduce effective playback buffer size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the buffer_get_free pcm_ops function to reduce the effective playback buffer size. All intermediate audio playback buffers become temporary buffers. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-11-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/paaudio.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/audio/paaudio.c b/audio/paaudio.c index 9df1e69c08..d94f858ec7 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -201,13 +201,11 @@ unlock_and_fail: return 0; } -static void *qpa_get_buffer_out(HWVoiceOut *hw, size_t *size) +static size_t qpa_buffer_get_free(HWVoiceOut *hw) { - PAVoiceOut *p = (PAVoiceOut *) hw; + PAVoiceOut *p = (PAVoiceOut *)hw; PAConnection *c = p->g->conn; - void *ret; size_t l; - int r; pa_threaded_mainloop_lock(c->mainloop); @@ -216,7 +214,6 @@ static void *qpa_get_buffer_out(HWVoiceOut *hw, size_t *size) if (pa_stream_get_state(p->stream) != PA_STREAM_READY) { /* wait for stream to become ready */ l = 0; - ret = NULL; goto unlock; } @@ -224,16 +221,33 @@ static void *qpa_get_buffer_out(HWVoiceOut *hw, size_t *size) CHECK_SUCCESS_GOTO(c, l != (size_t) -1, unlock_and_fail, "pa_stream_writable_size failed\n"); +unlock: + pa_threaded_mainloop_unlock(c->mainloop); + return l; + +unlock_and_fail: + pa_threaded_mainloop_unlock(c->mainloop); + return 0; +} + +static void *qpa_get_buffer_out(HWVoiceOut *hw, size_t *size) +{ + PAVoiceOut *p = (PAVoiceOut *)hw; + PAConnection *c = p->g->conn; + void *ret; + int r; + + pa_threaded_mainloop_lock(c->mainloop); + + CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail, + "pa_threaded_mainloop_lock failed\n"); + *size = -1; r = pa_stream_begin_write(p->stream, &ret, size); CHECK_SUCCESS_GOTO(c, r >= 0, unlock_and_fail, "pa_stream_begin_write failed\n"); -unlock: pa_threaded_mainloop_unlock(c->mainloop); - if (*size > l) { - *size = l; - } return ret; unlock_and_fail: @@ -901,6 +915,7 @@ static struct audio_pcm_ops qpa_pcm_ops = { .init_out = qpa_init_out, .fini_out = qpa_fini_out, .write = qpa_write, + .buffer_get_free = qpa_buffer_get_free, .get_buffer_out = qpa_get_buffer_out, .put_buffer_out = qpa_put_buffer_out, .volume_out = qpa_volume_out, From c93a593372967b98438ab832b1b6eded12fdee8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:08 +0100 Subject: [PATCH 19/35] dsoundaudio: reduce effective playback buffer size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the buffer_get_free pcm_ops function to reduce the effective playback buffer size. All intermediate audio playback buffers become temporary buffers. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-12-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/dsoundaudio.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c index 3dd2c4d4a6..231f3e65b3 100644 --- a/audio/dsoundaudio.c +++ b/audio/dsoundaudio.c @@ -427,22 +427,18 @@ static void dsound_enable_out(HWVoiceOut *hw, bool enable) } } -static void *dsound_get_buffer_out(HWVoiceOut *hw, size_t *size) +static size_t dsound_buffer_get_free(HWVoiceOut *hw) { DSoundVoiceOut *ds = (DSoundVoiceOut *) hw; LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer; HRESULT hr; - DWORD ppos, wpos, act_size; - size_t req_size; - int err; - void *ret; + DWORD ppos, wpos; hr = IDirectSoundBuffer_GetCurrentPosition( dsb, &ppos, ds->first_time ? &wpos : NULL); if (FAILED(hr)) { dsound_logerr(hr, "Could not get playback buffer position\n"); - *size = 0; - return NULL; + return 0; } if (ds->first_time) { @@ -450,13 +446,20 @@ static void *dsound_get_buffer_out(HWVoiceOut *hw, size_t *size) ds->first_time = false; } - req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul); - req_size = MIN(req_size, hw->size_emul - hw->pos_emul); + return audio_ring_dist(ppos, hw->pos_emul, hw->size_emul); +} - if (req_size == 0) { - *size = 0; - return NULL; - } +static void *dsound_get_buffer_out(HWVoiceOut *hw, size_t *size) +{ + DSoundVoiceOut *ds = (DSoundVoiceOut *)hw; + LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer; + DWORD act_size; + size_t req_size; + int err; + void *ret; + + req_size = MIN(*size, hw->size_emul - hw->pos_emul); + assert(req_size > 0); err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, &ret, NULL, &act_size, NULL, false, ds->s); @@ -699,6 +702,7 @@ static struct audio_pcm_ops dsound_pcm_ops = { .init_out = dsound_init_out, .fini_out = dsound_fini_out, .write = audio_generic_write, + .buffer_get_free = dsound_buffer_get_free, .get_buffer_out = dsound_get_buffer_out, .put_buffer_out = dsound_put_buffer_out, .enable_out = dsound_enable_out, From 385211e8f903c75d7d1322ba0e21c370b2edaddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:09 +0100 Subject: [PATCH 20/35] ossaudio: reduce effective playback buffer size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return the free buffer size for the mmapped case in function oss_buffer_get_free() to reduce the effective playback buffer size. All intermediate audio playback buffers become temporary buffers. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-13-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/ossaudio.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/audio/ossaudio.c b/audio/ossaudio.c index 1bd6800840..da9c232222 100644 --- a/audio/ossaudio.c +++ b/audio/ossaudio.c @@ -394,7 +394,7 @@ static size_t oss_buffer_get_free(HWVoiceOut *hw) OSSVoiceOut *oss = (OSSVoiceOut *)hw; if (oss->mmapped) { - return INT_MAX; + return oss_get_available_bytes(oss); } else { return audio_generic_buffer_get_free(hw); } @@ -402,9 +402,10 @@ static size_t oss_buffer_get_free(HWVoiceOut *hw) static void *oss_get_buffer_out(HWVoiceOut *hw, size_t *size) { - OSSVoiceOut *oss = (OSSVoiceOut *) hw; + OSSVoiceOut *oss = (OSSVoiceOut *)hw; + if (oss->mmapped) { - *size = MIN(oss_get_available_bytes(oss), hw->size_emul - hw->pos_emul); + *size = hw->size_emul - hw->pos_emul; return hw->buf_emul + hw->pos_emul; } else { return audio_generic_get_buffer_out(hw, size); From acf7a705980d157735242cf3bedabd2ef22c5384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:10 +0100 Subject: [PATCH 21/35] paaudio: fix samples vs. frames mix-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the mixing buffer size no longer adds to playback latency, fix the samples vs. frames mix-up in the mixing buffer size calculation. This change will go largely unnoticed as long as the user doesn't use a buffer-size smaller than timer-period. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-14-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/paaudio.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/audio/paaudio.c b/audio/paaudio.c index d94f858ec7..a53ed85e0b 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -549,11 +549,8 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as, } audio_pcm_init_info (&hw->info, &obt_as); - /* - * This is wrong. hw->samples counts in frames. hw->samples will be - * number of channels times larger than expected. - */ - hw->samples = audio_buffer_samples( + /* hw->samples counts in frames */ + hw->samples = audio_buffer_frames( qapi_AudiodevPaPerDirectionOptions_base(ppdo), &obt_as, 46440); return 0; @@ -601,11 +598,8 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque) } audio_pcm_init_info (&hw->info, &obt_as); - /* - * This is wrong. hw->samples counts in frames. hw->samples will be - * number of channels times larger than expected. - */ - hw->samples = audio_buffer_samples( + /* hw->samples counts in frames */ + hw->samples = audio_buffer_frames( qapi_AudiodevPaPerDirectionOptions_base(ppdo), &obt_as, 46440); return 0; From 7b672528070ffe9c401d760c0edb75849d8e484e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Tue, 1 Mar 2022 20:13:11 +0100 Subject: [PATCH 22/35] sdlaudio: fix samples vs. frames mix-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the same samples vs. frames mix-up that the previous commit fixed for the PulseAudio backend. Signed-off-by: Volker Rümelin Message-Id: <20220301191311.26695-15-vr_qemu@t-online.de> Signed-off-by: Gerd Hoffmann --- audio/sdlaudio.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c index e605c787ba..797b47bbdd 100644 --- a/audio/sdlaudio.c +++ b/audio/sdlaudio.c @@ -347,11 +347,8 @@ static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as, req.freq = as->freq; req.format = aud_to_sdlfmt (as->fmt); req.channels = as->nchannels; - /* - * This is wrong. SDL samples are QEMU frames. The buffer size will be - * the requested buffer size multiplied by the number of channels. - */ - req.samples = audio_buffer_samples( + /* SDL samples are QEMU frames */ + req.samples = audio_buffer_frames( qapi_AudiodevSdlPerDirectionOptions_base(spdo), as, 11610); req.callback = sdl_callback_out; req.userdata = sdl; From fedc1c19155f78f4f5be58092cd54cef0ea874ff Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Sat, 26 Feb 2022 18:07:15 +0000 Subject: [PATCH 23/35] hw/usb/redirect.c: Stop using qemu_oom_check() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qemu_oom_check() is a function which essentially says "if you pass me a NULL pointer then print a message then abort()". On POSIX systems the message includes strerror(errno); on Windows it includes the GetLastError() error value printed as an integer. Other than in the implementation of qemu_memalign(), we use this function only in hw/usb/redirect.c, for three checks: * on a call to usbredirparser_create() * on a call to usberedirparser_serialize() * on a call to malloc() The usbredir library API functions make no guarantees that they will set errno on errors, let alone that they might set the Windows-specific GetLastError string. malloc() is documented as setting errno, not GetLastError -- and in any case the only thing it might set errno to is ENOMEM. So qemu_oom_check() isn't the right thing for any of these. Replace them with straightforward error-checking code. This will allow us to get rid of qemu_oom_check(). Signed-off-by: Peter Maydell Reviewed-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20220226180723.1706285-2-peter.maydell@linaro.org> Signed-off-by: Gerd Hoffmann --- hw/usb/redirect.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 5f0ef9cb3b..8692ea2561 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -1239,7 +1239,11 @@ static void usbredir_create_parser(USBRedirDevice *dev) DPRINTF("creating usbredirparser\n"); - dev->parser = qemu_oom_check(usbredirparser_create()); + dev->parser = usbredirparser_create(); + if (!dev->parser) { + error_report("usbredirparser_create() failed"); + exit(1); + } dev->parser->priv = dev; dev->parser->log_func = usbredir_log; dev->parser->read_func = usbredir_read; @@ -2239,7 +2243,10 @@ static int usbredir_put_parser(QEMUFile *f, void *priv, size_t unused, } usbredirparser_serialize(dev->parser, &data, &len); - qemu_oom_check(data); + if (!data) { + error_report("usbredirparser_serialize failed"); + exit(1); + } qemu_put_be32(f, len); qemu_put_buffer(f, data, len); @@ -2330,7 +2337,11 @@ static int usbredir_get_bufpq(QEMUFile *f, void *priv, size_t unused, bufp->len = qemu_get_be32(f); bufp->status = qemu_get_be32(f); bufp->offset = 0; - bufp->data = qemu_oom_check(malloc(bufp->len)); /* regular malloc! */ + bufp->data = malloc(bufp->len); /* regular malloc! */ + if (!bufp->data) { + error_report("usbredir_get_bufpq: out of memory"); + exit(1); + } bufp->free_on_destroy = bufp->data; qemu_get_buffer(f, bufp->data, bufp->len); QTAILQ_INSERT_TAIL(&endp->bufpq, bufp, next); From bd7819de227f47df72e21558c019147673726a74 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 26 Feb 2022 20:59:53 +0900 Subject: [PATCH 24/35] coreaudio: Notify error in coreaudio_init_out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise, the audio subsystem tries to use the voice and eventually aborts due to the maximum number of samples in the buffer is not set. Signed-off-by: Akihiko Odaki Reviewed-by: Christian Schoenebeck Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20220226115953.60335-1-akihiko.odaki@gmail.com> Signed-off-by: Gerd Hoffmann --- audio/coreaudio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/audio/coreaudio.c b/audio/coreaudio.c index 91ea6ae975..0f19d0ce01 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -603,6 +603,8 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as, coreaudio_playback_logerr(status, "Could not remove voice property change listener\n"); } + + return -1; } return 0; From 64915058e18c2fbff8c80f9ad2e5be854dee2965 Mon Sep 17 00:00:00 2001 From: Dov Murik Date: Tue, 22 Feb 2022 07:19:05 +0000 Subject: [PATCH 25/35] hw/i386: Improve bounds checking in OVMF table parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When pc_system_parse_ovmf_flash() parses the optional GUIDed table in the end of the OVMF flash memory area, the table length field is checked for sizes that are too small, but doesn't error on sizes that are too big (bigger than the flash content itself). Add a check for maximal size of the OVMF table, and add an error report in case the size is invalid. In such a case, an error like this will be displayed during launch: qemu-system-x86_64: OVMF table has invalid size 4047 and the table parsing is skipped. Signed-off-by: Dov Murik Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Daniel P. Berrangé Message-Id: <20220222071906.2632426-2-dovmurik@linux.ibm.com> Signed-off-by: Gerd Hoffmann --- hw/i386/pc_sysfw_ovmf.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/i386/pc_sysfw_ovmf.c b/hw/i386/pc_sysfw_ovmf.c index f4dd92c588..df15c9737b 100644 --- a/hw/i386/pc_sysfw_ovmf.c +++ b/hw/i386/pc_sysfw_ovmf.c @@ -24,6 +24,7 @@ */ #include "qemu/osdep.h" +#include "qemu/error-report.h" #include "hw/i386/pc.h" #include "cpu.h" @@ -66,7 +67,13 @@ void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size) ptr -= sizeof(uint16_t); tot_len = le16_to_cpu(*(uint16_t *)ptr) - sizeof(guid) - sizeof(uint16_t); - if (tot_len <= 0) { + if (tot_len < 0 || tot_len > (ptr - flash_ptr)) { + error_report("OVMF table has invalid size %d", tot_len); + return; + } + + if (tot_len == 0) { + /* no entries in the OVMF table */ return; } From bfc8c14459dae16b525cd024600d20f022b6ee55 Mon Sep 17 00:00:00 2001 From: Dov Murik Date: Tue, 22 Feb 2022 07:19:06 +0000 Subject: [PATCH 26/35] hw/i386: Replace magic number with field length calculation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replce the literal magic number 48 with length calculation (32 bytes at the end of the firmware after the table footer + 16 bytes of the OVMF table footer GUID). No functional change intended. Signed-off-by: Dov Murik Reviewed-by: Daniel P. Berrangé Message-Id: <20220222071906.2632426-3-dovmurik@linux.ibm.com> Signed-off-by: Gerd Hoffmann --- hw/i386/pc_sysfw_ovmf.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/i386/pc_sysfw_ovmf.c b/hw/i386/pc_sysfw_ovmf.c index df15c9737b..07a4c267fa 100644 --- a/hw/i386/pc_sysfw_ovmf.c +++ b/hw/i386/pc_sysfw_ovmf.c @@ -30,6 +30,8 @@ #define OVMF_TABLE_FOOTER_GUID "96b582de-1fb2-45f7-baea-a366c55a082d" +static const int bytes_after_table_footer = 32; + static bool ovmf_flash_parsed; static uint8_t *ovmf_table; static int ovmf_table_len; @@ -53,12 +55,13 @@ void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size) /* * if this is OVMF there will be a table footer - * guid 48 bytes before the end of the flash file. If it's - * not found, silently abort the flash parsing. + * guid 48 bytes before the end of the flash file + * (= 32 bytes after the table + 16 bytes the GUID itself). + * If it's not found, silently abort the flash parsing. */ qemu_uuid_parse(OVMF_TABLE_FOOTER_GUID, &guid); guid = qemu_uuid_bswap(guid); /* guids are LE */ - ptr = flash_ptr + flash_size - 48; + ptr = flash_ptr + flash_size - (bytes_after_table_footer + sizeof(guid)); if (!qemu_uuid_is_equal((QemuUUID *)ptr, &guid)) { return; } From 0a2a40da4fba8256bffd9abd94626106f80c3c8c Mon Sep 17 00:00:00 2001 From: Dov Murik Date: Mon, 3 Jan 2022 11:14:13 +0200 Subject: [PATCH 27/35] docs: Add spec of OVMF GUIDed table for SEV guests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add docs/specs/sev-guest-firmware.rst which describes the GUIDed table in the end of OVMF's image which is parsed by QEMU, and currently used to describe some values for SEV and SEV-ES guests. Signed-off-by: Dov Murik Reviewed-by: Daniel P. Berrangé Message-Id: <20220103091413.2869-1-dovmurik@linux.ibm.com> Signed-off-by: Gerd Hoffmann --- docs/specs/index.rst | 1 + docs/specs/sev-guest-firmware.rst | 125 ++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 docs/specs/sev-guest-firmware.rst diff --git a/docs/specs/index.rst b/docs/specs/index.rst index ecc43896bb..2a35700fb3 100644 --- a/docs/specs/index.rst +++ b/docs/specs/index.rst @@ -18,3 +18,4 @@ guest hardware that is specific to QEMU. acpi_mem_hotplug acpi_pci_hotplug acpi_nvdimm + sev-guest-firmware diff --git a/docs/specs/sev-guest-firmware.rst b/docs/specs/sev-guest-firmware.rst new file mode 100644 index 0000000000..3f7f082df5 --- /dev/null +++ b/docs/specs/sev-guest-firmware.rst @@ -0,0 +1,125 @@ +==================================================== +QEMU/Guest Firmware Interface for AMD SEV and SEV-ES +==================================================== + +Overview +======== + +The guest firmware image (OVMF) may contain some configuration entries +which are used by QEMU before the guest launches. These are listed in a +GUIDed table at a known location in the firmware image. QEMU parses +this table when it loads the firmware image into memory, and then QEMU +reads individual entries when their values are needed. + +Though nothing in the table structure is SEV-specific, currently all the +entries in the table are related to SEV and SEV-ES features. + + +Table parsing in QEMU +--------------------- + +The table is parsed from the footer: first the presence of the table +footer GUID (96b582de-1fb2-45f7-baea-a366c55a082d) at 0xffffffd0 is +verified. If that is found, two bytes at 0xffffffce are the entire +table length. + +Then the table is scanned backwards looking for the specific entry GUID. + +QEMU files related to parsing and scanning the OVMF table: + - ``hw/i386/pc_sysfw_ovmf.c`` + +The edk2 firmware code that constructs this structure is in the +`OVMF Reset Vector file`_. + + +Table memory layout +------------------- + ++------------+--------+-----------------------------------------+ +| GPA | Length | Description | ++============+========+=========================================+ +| 0xffffff80 | 4 | Zero padding | ++------------+--------+-----------------------------------------+ +| 0xffffff84 | 4 | SEV hashes table base address | ++------------+--------+-----------------------------------------+ +| 0xffffff88 | 4 | SEV hashes table size (=0x400) | ++------------+--------+-----------------------------------------+ +| 0xffffff8c | 2 | SEV hashes table entry length (=0x1a) | ++------------+--------+-----------------------------------------+ +| 0xffffff8e | 16 | SEV hashes table GUID: | +| | | 7255371f-3a3b-4b04-927b-1da6efa8d454 | ++------------+--------+-----------------------------------------+ +| 0xffffff9e | 4 | SEV secret block base address | ++------------+--------+-----------------------------------------+ +| 0xffffffa2 | 4 | SEV secret block size (=0xc00) | ++------------+--------+-----------------------------------------+ +| 0xffffffa6 | 2 | SEV secret block entry length (=0x1a) | ++------------+--------+-----------------------------------------+ +| 0xffffffa8 | 16 | SEV secret block GUID: | +| | | 4c2eb361-7d9b-4cc3-8081-127c90d3d294 | ++------------+--------+-----------------------------------------+ +| 0xffffffb8 | 4 | SEV-ES AP reset RIP | ++------------+--------+-----------------------------------------+ +| 0xffffffbc | 2 | SEV-ES reset block entry length (=0x16) | ++------------+--------+-----------------------------------------+ +| 0xffffffbe | 16 | SEV-ES reset block entry GUID: | +| | | 00f771de-1a7e-4fcb-890e-68c77e2fb44e | ++------------+--------+-----------------------------------------+ +| 0xffffffce | 2 | Length of entire table including table | +| | | footer GUID and length (=0x72) | ++------------+--------+-----------------------------------------+ +| 0xffffffd0 | 16 | OVMF GUIDed table footer GUID: | +| | | 96b582de-1fb2-45f7-baea-a366c55a082d | ++------------+--------+-----------------------------------------+ +| 0xffffffe0 | 8 | Application processor entry point code | ++------------+--------+-----------------------------------------+ +| 0xffffffe8 | 8 | "\0\0\0\0VTF\0" | ++------------+--------+-----------------------------------------+ +| 0xfffffff0 | 16 | Reset vector code | ++------------+--------+-----------------------------------------+ + + +Table entries description +========================= + +SEV-ES reset block +------------------ + +Entry GUID: 00f771de-1a7e-4fcb-890e-68c77e2fb44e + +For the initial boot of an AP under SEV-ES, the "reset" RIP must be +programmed to the RAM area defined by this entry. The entry's format +is: + +* IP value [0:15] +* CS segment base [31:16] + +A hypervisor reads the CS segment base and IP value. The CS segment +base value represents the high order 16-bits of the CS segment base, so +the hypervisor must left shift the value of the CS segment base by 16 +bits to form the full CS segment base for the CS segment register. It +would then program the EIP register with the IP value as read. + + +SEV secret block +---------------- + +Entry GUID: 4c2eb361-7d9b-4cc3-8081-127c90d3d294 + +This describes the guest RAM area where the hypervisor should inject the +Guest Owner secret (using SEV_LAUNCH_SECRET). + + +SEV hashes table +---------------- + +Entry GUID: 7255371f-3a3b-4b04-927b-1da6efa8d454 + +This describes the guest RAM area where the hypervisor should install a +table describing the hashes of certain firmware configuration device +files that would otherwise be passed in unchecked. The current use is +for the kernel, initrd and command line values, but others may be added. + + +.. _OVMF Reset Vector file: + https://github.com/tianocore/edk2/blob/master/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm From a9fbce5e94d6b68ec8404f8a466fde873ba2bc73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 15 Feb 2022 00:13:35 +0400 Subject: [PATCH 28/35] ui/console: fix crash when using gl context with non-gl listeners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The commit 7cc712e98 ("ui: dispatch GL events to all listener") mechanically replaced the dpy_gl calls with a dispatch loop, using the same pre-conditions. However, it didn't take into account that all listeners do not have to implement the GL callbacks. Add the missing pre-conditions before calling the callbacks. Fix crash when running a GL-enabled VM with "-device virtio-gpu-gl-pci -display egl-headless -vnc :0". Fixes: 7cc712e98 ("ui: dispatch GL events to all listener") Reported-by: Akihiko Odaki Signed-off-by: Marc-André Lureau Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20220214201337.1814787-2-marcandre.lureau@redhat.com> Signed-off-by: Gerd Hoffmann --- ui/console.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/ui/console.c b/ui/console.c index 40eebb6d2c..79a01afd1e 100644 --- a/ui/console.c +++ b/ui/console.c @@ -1860,7 +1860,9 @@ void dpy_gl_scanout_disable(QemuConsole *con) con->scanout.kind = SCANOUT_NONE; } QLIST_FOREACH(dcl, &s->listeners, next) { - dcl->ops->dpy_gl_scanout_disable(dcl); + if (dcl->ops->dpy_gl_scanout_disable) { + dcl->ops->dpy_gl_scanout_disable(dcl); + } } } @@ -1881,10 +1883,12 @@ void dpy_gl_scanout_texture(QemuConsole *con, x, y, width, height }; QLIST_FOREACH(dcl, &s->listeners, next) { - dcl->ops->dpy_gl_scanout_texture(dcl, backing_id, - backing_y_0_top, - backing_width, backing_height, - x, y, width, height); + if (dcl->ops->dpy_gl_scanout_texture) { + dcl->ops->dpy_gl_scanout_texture(dcl, backing_id, + backing_y_0_top, + backing_width, backing_height, + x, y, width, height); + } } } @@ -1897,7 +1901,9 @@ void dpy_gl_scanout_dmabuf(QemuConsole *con, con->scanout.kind = SCANOUT_DMABUF; con->scanout.dmabuf = dmabuf; QLIST_FOREACH(dcl, &s->listeners, next) { - dcl->ops->dpy_gl_scanout_dmabuf(dcl, dmabuf); + if (dcl->ops->dpy_gl_scanout_dmabuf) { + dcl->ops->dpy_gl_scanout_dmabuf(dcl, dmabuf); + } } } @@ -1951,7 +1957,9 @@ void dpy_gl_update(QemuConsole *con, graphic_hw_gl_block(con, true); QLIST_FOREACH(dcl, &s->listeners, next) { - dcl->ops->dpy_gl_update(dcl, x, y, w, h); + if (dcl->ops->dpy_gl_update) { + dcl->ops->dpy_gl_update(dcl, x, y, w, h); + } } graphic_hw_gl_block(con, false); } From 6cdcf8810744cbc67074468b7f3c2f50f8e3539c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 15 Feb 2022 00:13:36 +0400 Subject: [PATCH 29/35] ui/console: fix texture leak when calling surface_gl_create_texture() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make surface_gl_create_texture() idempotent: if the surface is already bound to a texture, do not create a new one. This fixes texture leaks when there are multiple DBus listeners, for example. Reported-by: Akihiko Odaki Signed-off-by: Marc-André Lureau Message-Id: <20220214201337.1814787-3-marcandre.lureau@redhat.com> Signed-off-by: Gerd Hoffmann --- ui/console-gl.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/console-gl.c b/ui/console-gl.c index 7c9894a51d..8e3c9a3c8c 100644 --- a/ui/console-gl.c +++ b/ui/console-gl.c @@ -49,6 +49,10 @@ void surface_gl_create_texture(QemuGLShader *gls, assert(gls); assert(QEMU_IS_ALIGNED(surface_stride(surface), surface_bytes_per_pixel(surface))); + if (surface->texture) { + return; + } + switch (surface->format) { case PIXMAN_BE_b8g8r8x8: case PIXMAN_BE_b8g8r8a8: From cb8962c146b2633a4b04562281de9b2703bba849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 15 Feb 2022 00:13:37 +0400 Subject: [PATCH 30/35] ui: do not create a surface when resizing a GL scanout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qemu_console_resize() will create a blank surface and replace the current scanout with it if called while the current scanout is GL (texture or dmabuf). This is not only very costly, but also can produce glitches on the display/listener side. Instead, compare the current console size with the fitting console functions, which also works when the scanout is GL. Note: there might be still an unnecessary surface creation on calling qemu_console_resize() when the size is actually changing, but display backends currently rely on DisplaySurface details during dpy_gfx_switch() to handle various resize aspects. We would need more refactoring to handle resize without DisplaySurface, this is left for a future improvement. Signed-off-by: Marc-André Lureau Message-Id: <20220214201337.1814787-4-marcandre.lureau@redhat.com> Signed-off-by: Gerd Hoffmann --- ui/console.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ui/console.c b/ui/console.c index 79a01afd1e..365a2c14b8 100644 --- a/ui/console.c +++ b/ui/console.c @@ -2400,13 +2400,12 @@ static void vc_chr_open(Chardev *chr, void qemu_console_resize(QemuConsole *s, int width, int height) { - DisplaySurface *surface = qemu_console_surface(s); + DisplaySurface *surface; assert(s->console_type == GRAPHIC_CONSOLE); - if (surface && (surface->flags & QEMU_ALLOCATED_FLAG) && - pixman_image_get_width(surface->image) == width && - pixman_image_get_height(surface->image) == height) { + if (qemu_console_get_width(s, -1) == width && + qemu_console_get_height(s, -1) == height) { return; } From 02a8ee2e1840d13db065f418969bcfcb404a94cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 14 Feb 2022 15:59:17 +0400 Subject: [PATCH 31/35] ui/clipboard: fix use-after-free regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same info may be used to update the clipboard, and may be freed before being ref'ed again. Fixes: 70a54b01693ed ("ui: avoid compiler warnings from unused clipboard info variable") Signed-off-by: Marc-André Lureau Reviewed-by: Daniel P. Berrangé Message-Id: <20220214115917.1679568-1-marcandre.lureau@redhat.com> Signed-off-by: Gerd Hoffmann --- ui/clipboard.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/clipboard.c b/ui/clipboard.c index 5f15cf853d..9079ef829b 100644 --- a/ui/clipboard.c +++ b/ui/clipboard.c @@ -66,8 +66,10 @@ void qemu_clipboard_update(QemuClipboardInfo *info) notifier_list_notify(&clipboard_notifiers, ¬ify); - qemu_clipboard_info_unref(cbinfo[info->selection]); - cbinfo[info->selection] = qemu_clipboard_info_ref(info); + if (cbinfo[info->selection] != info) { + qemu_clipboard_info_unref(cbinfo[info->selection]); + cbinfo[info->selection] = qemu_clipboard_info_ref(info); + } } QemuClipboardInfo *qemu_clipboard_info(QemuClipboardSelection selection) From 5b6988c18a57fa72ddd44a232388518fcf8124bc Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 14 Feb 2022 18:13:20 +0900 Subject: [PATCH 32/35] ui/cocoa: Add Services menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Services menu functionality of Cocoa is described at: https://developer.apple.com/design/human-interface-guidelines/macos/extensions/services/ Signed-off-by: Akihiko Odaki Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Message-Id: <20220214091320.51750-1-akihiko.odaki@gmail.com> Signed-off-by: Gerd Hoffmann --- ui/cocoa.m | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/cocoa.m b/ui/cocoa.m index b6e70e9134..8ab9ab5e84 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -1611,11 +1611,15 @@ static void create_initial_menus(void) NSMenuItem *menuItem; [NSApp setMainMenu:[[NSMenu alloc] init]]; + [NSApp setServicesMenu:[[NSMenu alloc] initWithTitle:@"Services"]]; // Application menu menu = [[NSMenu alloc] initWithTitle:@""]; [menu addItemWithTitle:@"About QEMU" action:@selector(do_about_menu_item:) keyEquivalent:@""]; // About QEMU [menu addItem:[NSMenuItem separatorItem]]; //Separator + menuItem = [menu addItemWithTitle:@"Services" action:nil keyEquivalent:@""]; + [menuItem setSubmenu:[NSApp servicesMenu]]; + [menu addItem:[NSMenuItem separatorItem]]; [menu addItemWithTitle:@"Hide QEMU" action:@selector(hide:) keyEquivalent:@"h"]; //Hide QEMU menuItem = (NSMenuItem *)[menu addItemWithTitle:@"Hide Others" action:@selector(hideOtherApplications:) keyEquivalent:@"h"]; // Hide Others [menuItem setKeyEquivalentModifierMask:(NSEventModifierFlagOption|NSEventModifierFlagCommand)]; From f4ba24b3857251e7531803d6a157b3495dcb5703 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 13 Feb 2022 11:18:00 +0900 Subject: [PATCH 33/35] softmmu/qdev-monitor: Add virtio-gpu-gl aliases Signed-off-by: Akihiko Odaki Message-Id: <20220213021800.2525-1-akihiko.odaki@gmail.com> Signed-off-by: Gerd Hoffmann --- softmmu/qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 01f3834db5..a0df820b9d 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -83,6 +83,8 @@ static const QDevAlias qdev_alias_table[] = { { "virtio-gpu-device", "virtio-gpu", QEMU_ARCH_VIRTIO_MMIO }, { "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_VIRTIO_CCW }, { "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_VIRTIO_PCI }, + { "virtio-gpu-gl-device", "virtio-gpu-gl", QEMU_ARCH_VIRTIO_MMIO }, + { "virtio-gpu-gl-pci", "virtio-gpu-gl", QEMU_ARCH_VIRTIO_PCI }, { "virtio-input-host-device", "virtio-input-host", QEMU_ARCH_VIRTIO_MMIO }, { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_VIRTIO_CCW }, { "virtio-input-host-pci", "virtio-input-host", QEMU_ARCH_VIRTIO_PCI }, From 4377683df969e715e3cb2dbd258e44f9ff51f788 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 13 Feb 2022 11:15:29 +0900 Subject: [PATCH 34/35] edid: Fix clock of Detailed Timing Descriptor The clock field is 16-bits in EDID Detailed Timing Descriptor, but edid_desc_timing assumed it is 32-bit. Write the 16-bit value if it fits in 16-bit. Write DisplayID otherwise. Signed-off-by: Akihiko Odaki Message-Id: <20220213021529.2248-1-akihiko.odaki@gmail.com> Signed-off-by: Gerd Hoffmann --- hw/display/edid-generate.c | 66 ++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c index bccf32af69..2cb819675e 100644 --- a/hw/display/edid-generate.c +++ b/hw/display/edid-generate.c @@ -255,33 +255,31 @@ static void edid_desc_dummy(uint8_t *desc) edid_desc_type(desc, 0x10); } -static void edid_desc_timing(uint8_t *desc, uint32_t refresh_rate, +static void edid_desc_timing(uint8_t *desc, const Timings *timings, uint32_t xres, uint32_t yres, uint32_t xmm, uint32_t ymm) { - Timings timings; - generate_timings(&timings, refresh_rate, xres, yres); - stl_le_p(desc, timings.clock); + stw_le_p(desc, timings->clock); desc[2] = xres & 0xff; - desc[3] = timings.xblank & 0xff; + desc[3] = timings->xblank & 0xff; desc[4] = (((xres & 0xf00) >> 4) | - ((timings.xblank & 0xf00) >> 8)); + ((timings->xblank & 0xf00) >> 8)); desc[5] = yres & 0xff; - desc[6] = timings.yblank & 0xff; + desc[6] = timings->yblank & 0xff; desc[7] = (((yres & 0xf00) >> 4) | - ((timings.yblank & 0xf00) >> 8)); + ((timings->yblank & 0xf00) >> 8)); - desc[8] = timings.xfront & 0xff; - desc[9] = timings.xsync & 0xff; + desc[8] = timings->xfront & 0xff; + desc[9] = timings->xsync & 0xff; - desc[10] = (((timings.yfront & 0x00f) << 4) | - ((timings.ysync & 0x00f) << 0)); - desc[11] = (((timings.xfront & 0x300) >> 2) | - ((timings.xsync & 0x300) >> 4) | - ((timings.yfront & 0x030) >> 2) | - ((timings.ysync & 0x030) >> 4)); + desc[10] = (((timings->yfront & 0x00f) << 4) | + ((timings->ysync & 0x00f) << 0)); + desc[11] = (((timings->xfront & 0x300) >> 2) | + ((timings->xsync & 0x300) >> 4) | + ((timings->yfront & 0x030) >> 2) | + ((timings->ysync & 0x030) >> 4)); desc[12] = xmm & 0xff; desc[13] = ymm & 0xff; @@ -348,13 +346,10 @@ static void init_displayid(uint8_t *did) edid_checksum(did + 1, did[2] + 4); } -static void qemu_displayid_generate(uint8_t *did, uint32_t refresh_rate, +static void qemu_displayid_generate(uint8_t *did, const Timings *timings, uint32_t xres, uint32_t yres, uint32_t xmm, uint32_t ymm) { - Timings timings; - generate_timings(&timings, refresh_rate, xres, yres); - did[0] = 0x70; /* display id extension */ did[1] = 0x13; /* version 1.3 */ did[2] = 23; /* length */ @@ -364,21 +359,21 @@ static void qemu_displayid_generate(uint8_t *did, uint32_t refresh_rate, did[6] = 0x00; /* revision */ did[7] = 0x14; /* block length */ - did[8] = timings.clock & 0xff; - did[9] = (timings.clock & 0xff00) >> 8; - did[10] = (timings.clock & 0xff0000) >> 16; + did[8] = timings->clock & 0xff; + did[9] = (timings->clock & 0xff00) >> 8; + did[10] = (timings->clock & 0xff0000) >> 16; did[11] = 0x88; /* leave aspect ratio undefined */ stw_le_p(did + 12, 0xffff & (xres - 1)); - stw_le_p(did + 14, 0xffff & (timings.xblank - 1)); - stw_le_p(did + 16, 0xffff & (timings.xfront - 1)); - stw_le_p(did + 18, 0xffff & (timings.xsync - 1)); + stw_le_p(did + 14, 0xffff & (timings->xblank - 1)); + stw_le_p(did + 16, 0xffff & (timings->xfront - 1)); + stw_le_p(did + 18, 0xffff & (timings->xsync - 1)); stw_le_p(did + 20, 0xffff & (yres - 1)); - stw_le_p(did + 22, 0xffff & (timings.yblank - 1)); - stw_le_p(did + 24, 0xffff & (timings.yfront - 1)); - stw_le_p(did + 26, 0xffff & (timings.ysync - 1)); + stw_le_p(did + 22, 0xffff & (timings->yblank - 1)); + stw_le_p(did + 24, 0xffff & (timings->yfront - 1)); + stw_le_p(did + 26, 0xffff & (timings->ysync - 1)); edid_checksum(did + 1, did[2] + 4); } @@ -386,6 +381,7 @@ static void qemu_displayid_generate(uint8_t *did, uint32_t refresh_rate, void qemu_edid_generate(uint8_t *edid, size_t size, qemu_edid_info *info) { + Timings timings; uint8_t *desc = edid + 54; uint8_t *xtra3 = NULL; uint8_t *dta = NULL; @@ -409,9 +405,6 @@ void qemu_edid_generate(uint8_t *edid, size_t size, if (!info->prefy) { info->prefy = 800; } - if (info->prefx >= 4096 || info->prefy >= 4096) { - large_screen = 1; - } if (info->width_mm && info->height_mm) { width_mm = info->width_mm; height_mm = info->height_mm; @@ -421,6 +414,11 @@ void qemu_edid_generate(uint8_t *edid, size_t size, height_mm = qemu_edid_dpi_to_mm(dpi, info->prefy); } + generate_timings(&timings, refresh_rate, info->prefx, info->prefy); + if (info->prefx >= 4096 || info->prefy >= 4096 || timings.clock >= 65536) { + large_screen = 1; + } + /* =============== extensions =============== */ if (size >= 256) { @@ -501,7 +499,7 @@ void qemu_edid_generate(uint8_t *edid, size_t size, if (!large_screen) { /* The DTD section has only 12 bits to store the resolution */ - edid_desc_timing(desc, refresh_rate, info->prefx, info->prefy, + edid_desc_timing(desc, &timings, info->prefx, info->prefy, width_mm, height_mm); desc = edid_desc_next(edid, dta, desc); } @@ -536,7 +534,7 @@ void qemu_edid_generate(uint8_t *edid, size_t size, /* =============== display id extensions =============== */ if (did && large_screen) { - qemu_displayid_generate(did, refresh_rate, info->prefx, info->prefy, + qemu_displayid_generate(did, &timings, info->prefx, info->prefy, width_mm, height_mm); } From 02218aedb1c851340207db89b8eeb96843fed241 Mon Sep 17 00:00:00 2001 From: Carwyn Ellis Date: Sun, 6 Feb 2022 18:39:55 +0000 Subject: [PATCH 35/35] hw/display/vmware_vga: replace fprintf calls with trace events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Debug output was always being sent to STDERR. This has been replaced with trace events. Signed-off-by: Carwyn Ellis Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20220206183956.10694-2-carwynellis@gmail.com> Signed-off-by: Gerd Hoffmann --- hw/display/trace-events | 3 +++ hw/display/vmware_vga.c | 30 ++++++++++++++++++------------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/hw/display/trace-events b/hw/display/trace-events index 4a687d1b8e..91efc88f04 100644 --- a/hw/display/trace-events +++ b/hw/display/trace-events @@ -21,6 +21,9 @@ vmware_palette_write(uint32_t index, uint32_t value) "index %d, value 0x%x" vmware_scratch_read(uint32_t index, uint32_t value) "index %d, value 0x%x" vmware_scratch_write(uint32_t index, uint32_t value) "index %d, value 0x%x" vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @ %d bpp" +vmware_verify_rect_less_than_zero(const char *name, const char *param, int x) "%s: %s was < 0 (%d)" +vmware_verify_rect_greater_than_bound(const char *name, const char *param, int bound, int x) "%s: %s was > %d (%d)" +vmware_verify_rect_surface_bound_exceeded(const char *name, const char *component, int bound, const char *param1, int value1, const char *param2, int value2) "%s: %s > %d (%s: %d, %s: %d)" # virtio-gpu-base.c virtio_gpu_features(bool virgl) "virgl %d" diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index e2969a6c81..0cc43a1f15 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -297,46 +297,52 @@ static inline bool vmsvga_verify_rect(DisplaySurface *surface, int x, int y, int w, int h) { if (x < 0) { - fprintf(stderr, "%s: x was < 0 (%d)\n", name, x); + trace_vmware_verify_rect_less_than_zero(name, "x", x); return false; } if (x > SVGA_MAX_WIDTH) { - fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x); + trace_vmware_verify_rect_greater_than_bound(name, "x", SVGA_MAX_WIDTH, + x); return false; } if (w < 0) { - fprintf(stderr, "%s: w was < 0 (%d)\n", name, w); + trace_vmware_verify_rect_less_than_zero(name, "w", w); return false; } if (w > SVGA_MAX_WIDTH) { - fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w); + trace_vmware_verify_rect_greater_than_bound(name, "w", SVGA_MAX_WIDTH, + w); return false; } if (x + w > surface_width(surface)) { - fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n", - name, surface_width(surface), x, w); + trace_vmware_verify_rect_surface_bound_exceeded(name, "width", + surface_width(surface), + "x", x, "w", w); return false; } if (y < 0) { - fprintf(stderr, "%s: y was < 0 (%d)\n", name, y); + trace_vmware_verify_rect_less_than_zero(name, "y", y); return false; } if (y > SVGA_MAX_HEIGHT) { - fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y); + trace_vmware_verify_rect_greater_than_bound(name, "y", SVGA_MAX_HEIGHT, + y); return false; } if (h < 0) { - fprintf(stderr, "%s: h was < 0 (%d)\n", name, h); + trace_vmware_verify_rect_less_than_zero(name, "h", h); return false; } if (h > SVGA_MAX_HEIGHT) { - fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h); + trace_vmware_verify_rect_greater_than_bound(name, "y", SVGA_MAX_HEIGHT, + y); return false; } if (y + h > surface_height(surface)) { - fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n", - name, surface_height(surface), y, h); + trace_vmware_verify_rect_surface_bound_exceeded(name, "height", + surface_height(surface), + "y", y, "h", h); return false; }