* [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
@ 2018-11-09 14:14 Gerd Hoffmann
2018-11-09 14:52 ` Liam Merwick
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2018-11-09 14:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Gerd Hoffmann
Broken (segfaultson first keypress) and appearently unused.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/hw/bt.h | 3 -
hw/bt/hid.c | 554 ----------------------------------------------------
vl.c | 34 +---
hw/bt/Makefile.objs | 3 +-
qemu-options.hx | 9 -
5 files changed, 2 insertions(+), 601 deletions(-)
delete mode 100644 hw/bt/hid.c
diff --git a/include/hw/bt.h b/include/hw/bt.h
index b5e11d4d43..73fb1911f6 100644
--- a/include/hw/bt.h
+++ b/include/hw/bt.h
@@ -173,9 +173,6 @@ enum bt_l2cap_psm_predef {
/* bt-sdp.c */
void bt_l2cap_sdp_init(struct bt_l2cap_device_s *dev);
-/* bt-hid.c */
-struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net);
-
/* Link Management Protocol layer defines */
#define LLID_ACLU_CONT 0x1
diff --git a/hw/bt/hid.c b/hw/bt/hid.c
deleted file mode 100644
index 056291f9b5..0000000000
--- a/hw/bt/hid.c
+++ /dev/null
@@ -1,554 +0,0 @@
-/*
- * QEMU Bluetooth HID Profile wrapper for USB HID.
- *
- * Copyright (C) 2007-2008 OpenMoko, Inc.
- * Written by Andrzej Zaborowski <andrew@openedhand.com>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 or
- * (at your option) version 3 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, if not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "qemu/osdep.h"
-#include "qemu-common.h"
-#include "qemu/timer.h"
-#include "ui/console.h"
-#include "hw/input/hid.h"
-#include "hw/bt.h"
-
-enum hid_transaction_req {
- BT_HANDSHAKE = 0x0,
- BT_HID_CONTROL = 0x1,
- BT_GET_REPORT = 0x4,
- BT_SET_REPORT = 0x5,
- BT_GET_PROTOCOL = 0x6,
- BT_SET_PROTOCOL = 0x7,
- BT_GET_IDLE = 0x8,
- BT_SET_IDLE = 0x9,
- BT_DATA = 0xa,
- BT_DATC = 0xb,
-};
-
-enum hid_transaction_handshake {
- BT_HS_SUCCESSFUL = 0x0,
- BT_HS_NOT_READY = 0x1,
- BT_HS_ERR_INVALID_REPORT_ID = 0x2,
- BT_HS_ERR_UNSUPPORTED_REQUEST = 0x3,
- BT_HS_ERR_INVALID_PARAMETER = 0x4,
- BT_HS_ERR_UNKNOWN = 0xe,
- BT_HS_ERR_FATAL = 0xf,
-};
-
-enum hid_transaction_control {
- BT_HC_NOP = 0x0,
- BT_HC_HARD_RESET = 0x1,
- BT_HC_SOFT_RESET = 0x2,
- BT_HC_SUSPEND = 0x3,
- BT_HC_EXIT_SUSPEND = 0x4,
- BT_HC_VIRTUAL_CABLE_UNPLUG = 0x5,
-};
-
-enum hid_protocol {
- BT_HID_PROTO_BOOT = 0,
- BT_HID_PROTO_REPORT = 1,
-};
-
-enum hid_boot_reportid {
- BT_HID_BOOT_INVALID = 0,
- BT_HID_BOOT_KEYBOARD,
- BT_HID_BOOT_MOUSE,
-};
-
-enum hid_data_pkt {
- BT_DATA_OTHER = 0,
- BT_DATA_INPUT,
- BT_DATA_OUTPUT,
- BT_DATA_FEATURE,
-};
-
-#define BT_HID_MTU 48
-
-/* HID interface requests */
-#define GET_REPORT 0xa101
-#define GET_IDLE 0xa102
-#define GET_PROTOCOL 0xa103
-#define SET_REPORT 0x2109
-#define SET_IDLE 0x210a
-#define SET_PROTOCOL 0x210b
-
-struct bt_hid_device_s {
- struct bt_l2cap_device_s btdev;
- struct bt_l2cap_conn_params_s *control;
- struct bt_l2cap_conn_params_s *interrupt;
- HIDState hid;
-
- int proto;
- int connected;
- int data_type;
- int intr_state;
- struct {
- int len;
- uint8_t buffer[1024];
- } dataother, datain, dataout, feature, intrdataout;
- enum {
- bt_state_ready,
- bt_state_transaction,
- bt_state_suspend,
- } state;
-};
-
-static void bt_hid_reset(struct bt_hid_device_s *s)
-{
- struct bt_scatternet_s *net = s->btdev.device.net;
-
- /* Go as far as... */
- bt_l2cap_device_done(&s->btdev);
- bt_l2cap_device_init(&s->btdev, net);
-
- hid_reset(&s->hid);
- s->proto = BT_HID_PROTO_REPORT;
- s->state = bt_state_ready;
- s->dataother.len = 0;
- s->datain.len = 0;
- s->dataout.len = 0;
- s->feature.len = 0;
- s->intrdataout.len = 0;
- s->intr_state = 0;
-}
-
-static int bt_hid_out(struct bt_hid_device_s *s)
-{
- if (s->data_type == BT_DATA_OUTPUT) {
- /* nothing */
- ;
- }
-
- if (s->data_type == BT_DATA_FEATURE) {
- /* XXX:
- * does this send a USB_REQ_CLEAR_FEATURE/USB_REQ_SET_FEATURE
- * or a SET_REPORT? */
- ;
- }
-
- return -1;
-}
-
-static int bt_hid_in(struct bt_hid_device_s *s)
-{
- s->datain.len = hid_keyboard_poll(&s->hid, s->datain.buffer,
- sizeof(s->datain.buffer));
- return s->datain.len;
-}
-
-static void bt_hid_send_handshake(struct bt_hid_device_s *s, int result)
-{
- *s->control->sdu_out(s->control, 1) =
- (BT_HANDSHAKE << 4) | result;
- s->control->sdu_submit(s->control);
-}
-
-static void bt_hid_send_control(struct bt_hid_device_s *s, int operation)
-{
- *s->control->sdu_out(s->control, 1) =
- (BT_HID_CONTROL << 4) | operation;
- s->control->sdu_submit(s->control);
-}
-
-static void bt_hid_disconnect(struct bt_hid_device_s *s)
-{
- /* Disconnect s->control and s->interrupt */
-}
-
-static void bt_hid_send_data(struct bt_l2cap_conn_params_s *ch, int type,
- const uint8_t *data, int len)
-{
- uint8_t *pkt, hdr = (BT_DATA << 4) | type;
- int plen;
-
- do {
- plen = MIN(len, ch->remote_mtu - 1);
- pkt = ch->sdu_out(ch, plen + 1);
-
- pkt[0] = hdr;
- if (plen)
- memcpy(pkt + 1, data, plen);
- ch->sdu_submit(ch);
-
- len -= plen;
- data += plen;
- hdr = (BT_DATC << 4) | type;
- } while (plen == ch->remote_mtu - 1);
-}
-
-static void bt_hid_control_transaction(struct bt_hid_device_s *s,
- const uint8_t *data, int len)
-{
- uint8_t type, parameter;
- int rlen, ret = -1;
- if (len < 1)
- return;
-
- type = data[0] >> 4;
- parameter = data[0] & 0xf;
-
- switch (type) {
- case BT_HANDSHAKE:
- case BT_DATA:
- switch (parameter) {
- default:
- /* These are not expected to be sent this direction. */
- ret = BT_HS_ERR_INVALID_PARAMETER;
- }
- break;
-
- case BT_HID_CONTROL:
- if (len != 1 || (parameter != BT_HC_VIRTUAL_CABLE_UNPLUG &&
- s->state == bt_state_transaction)) {
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- }
- switch (parameter) {
- case BT_HC_NOP:
- break;
- case BT_HC_HARD_RESET:
- case BT_HC_SOFT_RESET:
- bt_hid_reset(s);
- break;
- case BT_HC_SUSPEND:
- if (s->state == bt_state_ready)
- s->state = bt_state_suspend;
- else
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- case BT_HC_EXIT_SUSPEND:
- if (s->state == bt_state_suspend)
- s->state = bt_state_ready;
- else
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- case BT_HC_VIRTUAL_CABLE_UNPLUG:
- bt_hid_disconnect(s);
- break;
- default:
- ret = BT_HS_ERR_INVALID_PARAMETER;
- }
- break;
-
- case BT_GET_REPORT:
- /* No ReportIDs declared. */
- if (((parameter & 8) && len != 3) ||
- (!(parameter & 8) && len != 1) ||
- s->state != bt_state_ready) {
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- }
- if (parameter & 8)
- rlen = data[2] | (data[3] << 8);
- else
- rlen = INT_MAX;
- switch (parameter & 3) {
- case BT_DATA_OTHER:
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- case BT_DATA_INPUT:
- /* Here we can as well poll s->usbdev */
- bt_hid_send_data(s->control, BT_DATA_INPUT,
- s->datain.buffer, MIN(rlen, s->datain.len));
- break;
- case BT_DATA_OUTPUT:
- bt_hid_send_data(s->control, BT_DATA_OUTPUT,
- s->dataout.buffer, MIN(rlen, s->dataout.len));
- break;
- case BT_DATA_FEATURE:
- bt_hid_send_data(s->control, BT_DATA_FEATURE,
- s->feature.buffer, MIN(rlen, s->feature.len));
- break;
- }
- break;
-
- case BT_SET_REPORT:
- if (len < 2 || len > BT_HID_MTU || s->state != bt_state_ready ||
- (parameter & 3) == BT_DATA_OTHER ||
- (parameter & 3) == BT_DATA_INPUT) {
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- }
- s->data_type = parameter & 3;
- if (s->data_type == BT_DATA_OUTPUT) {
- s->dataout.len = len - 1;
- memcpy(s->dataout.buffer, data + 1, s->dataout.len);
- } else {
- s->feature.len = len - 1;
- memcpy(s->feature.buffer, data + 1, s->feature.len);
- }
- if (len == BT_HID_MTU)
- s->state = bt_state_transaction;
- else
- bt_hid_out(s);
- break;
-
- case BT_GET_PROTOCOL:
- if (len != 1 || s->state == bt_state_transaction) {
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- }
- *s->control->sdu_out(s->control, 1) = s->proto;
- s->control->sdu_submit(s->control);
- break;
-
- case BT_SET_PROTOCOL:
- if (len != 1 || s->state == bt_state_transaction ||
- (parameter != BT_HID_PROTO_BOOT &&
- parameter != BT_HID_PROTO_REPORT)) {
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- }
- s->proto = parameter;
- s->hid.protocol = parameter;
- ret = BT_HS_SUCCESSFUL;
- break;
-
- case BT_GET_IDLE:
- if (len != 1 || s->state == bt_state_transaction) {
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- }
- *s->control->sdu_out(s->control, 1) = s->hid.idle;
- s->control->sdu_submit(s->control);
- break;
-
- case BT_SET_IDLE:
- if (len != 2 || s->state == bt_state_transaction) {
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- }
-
- s->hid.idle = data[1];
- /* XXX: Does this generate a handshake? */
- break;
-
- case BT_DATC:
- if (len > BT_HID_MTU || s->state != bt_state_transaction) {
- ret = BT_HS_ERR_INVALID_PARAMETER;
- break;
- }
- if (s->data_type == BT_DATA_OUTPUT) {
- memcpy(s->dataout.buffer + s->dataout.len, data + 1, len - 1);
- s->dataout.len += len - 1;
- } else {
- memcpy(s->feature.buffer + s->feature.len, data + 1, len - 1);
- s->feature.len += len - 1;
- }
- if (len < BT_HID_MTU) {
- bt_hid_out(s);
- s->state = bt_state_ready;
- }
- break;
-
- default:
- ret = BT_HS_ERR_UNSUPPORTED_REQUEST;
- }
-
- if (ret != -1)
- bt_hid_send_handshake(s, ret);
-}
-
-static void bt_hid_control_sdu(void *opaque, const uint8_t *data, int len)
-{
- struct bt_hid_device_s *hid = opaque;
-
- bt_hid_control_transaction(hid, data, len);
-}
-
-static void bt_hid_datain(HIDState *hs)
-{
- struct bt_hid_device_s *hid =
- container_of(hs, struct bt_hid_device_s, hid);
-
- /* If suspended, wake-up and send a wake-up event first. We might
- * want to also inspect the input report and ignore event like
- * mouse movements until a button event occurs. */
- if (hid->state == bt_state_suspend) {
- hid->state = bt_state_ready;
- }
-
- if (bt_hid_in(hid) > 0)
- /* TODO: when in boot-mode precede any Input reports with the ReportID
- * byte, here and in GetReport/SetReport on the Control channel. */
- bt_hid_send_data(hid->interrupt, BT_DATA_INPUT,
- hid->datain.buffer, hid->datain.len);
-}
-
-static void bt_hid_interrupt_sdu(void *opaque, const uint8_t *data, int len)
-{
- struct bt_hid_device_s *hid = opaque;
-
- if (len > BT_HID_MTU || len < 1)
- goto bad;
- if ((data[0] & 3) != BT_DATA_OUTPUT)
- goto bad;
- if ((data[0] >> 4) == BT_DATA) {
- if (hid->intr_state)
- goto bad;
-
- hid->data_type = BT_DATA_OUTPUT;
- hid->intrdataout.len = 0;
- } else if ((data[0] >> 4) == BT_DATC) {
- if (!hid->intr_state)
- goto bad;
- } else
- goto bad;
-
- memcpy(hid->intrdataout.buffer + hid->intrdataout.len, data + 1, len - 1);
- hid->intrdataout.len += len - 1;
- hid->intr_state = (len == BT_HID_MTU);
- if (!hid->intr_state) {
- memcpy(hid->dataout.buffer, hid->intrdataout.buffer,
- hid->dataout.len = hid->intrdataout.len);
- bt_hid_out(hid);
- }
-
- return;
-bad:
- error_report("%s: bad transaction on Interrupt channel.",
- __func__);
-}
-
-/* "Virtual cable" plug/unplug event. */
-static void bt_hid_connected_update(struct bt_hid_device_s *hid)
-{
- int prev = hid->connected;
-
- hid->connected = hid->control && hid->interrupt;
-
- /* Stop page-/inquiry-scanning when a host is connected. */
- hid->btdev.device.page_scan = !hid->connected;
- hid->btdev.device.inquiry_scan = !hid->connected;
-
- if (hid->connected && !prev) {
- hid_reset(&hid->hid);
- hid->proto = BT_HID_PROTO_REPORT;
- }
-
- /* Should set HIDVirtualCable in SDP (possibly need to check that SDP
- * isn't destroyed yet, in case we're being called from handle_destroy) */
-}
-
-static void bt_hid_close_control(void *opaque)
-{
- struct bt_hid_device_s *hid = opaque;
-
- hid->control = NULL;
- bt_hid_connected_update(hid);
-}
-
-static void bt_hid_close_interrupt(void *opaque)
-{
- struct bt_hid_device_s *hid = opaque;
-
- hid->interrupt = NULL;
- bt_hid_connected_update(hid);
-}
-
-static int bt_hid_new_control_ch(struct bt_l2cap_device_s *dev,
- struct bt_l2cap_conn_params_s *params)
-{
- struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
-
- if (hid->control)
- return 1;
-
- hid->control = params;
- hid->control->opaque = hid;
- hid->control->close = bt_hid_close_control;
- hid->control->sdu_in = bt_hid_control_sdu;
-
- bt_hid_connected_update(hid);
-
- return 0;
-}
-
-static int bt_hid_new_interrupt_ch(struct bt_l2cap_device_s *dev,
- struct bt_l2cap_conn_params_s *params)
-{
- struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
-
- if (hid->interrupt)
- return 1;
-
- hid->interrupt = params;
- hid->interrupt->opaque = hid;
- hid->interrupt->close = bt_hid_close_interrupt;
- hid->interrupt->sdu_in = bt_hid_interrupt_sdu;
-
- bt_hid_connected_update(hid);
-
- return 0;
-}
-
-static void bt_hid_destroy(struct bt_device_s *dev)
-{
- struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
-
- if (hid->connected)
- bt_hid_send_control(hid, BT_HC_VIRTUAL_CABLE_UNPLUG);
- bt_l2cap_device_done(&hid->btdev);
-
- hid_free(&hid->hid);
-
- g_free(hid);
-}
-
-enum peripheral_minor_class {
- class_other = 0 << 4,
- class_keyboard = 1 << 4,
- class_pointing = 2 << 4,
- class_combo = 3 << 4,
-};
-
-static struct bt_device_s *bt_hid_init(struct bt_scatternet_s *net,
- enum peripheral_minor_class minor)
-{
- struct bt_hid_device_s *s = g_malloc0(sizeof(*s));
- uint32_t class =
- /* Format type */
- (0 << 0) |
- /* Device class */
- (minor << 2) |
- (5 << 8) | /* "Peripheral" */
- /* Service classes */
- (1 << 13) | /* Limited discoverable mode */
- (1 << 19); /* Capturing device (?) */
-
- bt_l2cap_device_init(&s->btdev, net);
- bt_l2cap_sdp_init(&s->btdev);
- bt_l2cap_psm_register(&s->btdev, BT_PSM_HID_CTRL,
- BT_HID_MTU, bt_hid_new_control_ch);
- bt_l2cap_psm_register(&s->btdev, BT_PSM_HID_INTR,
- BT_HID_MTU, bt_hid_new_interrupt_ch);
-
- hid_init(&s->hid, HID_KEYBOARD, bt_hid_datain);
- s->btdev.device.lmp_name = "BT Keyboard";
-
- s->btdev.device.handle_destroy = bt_hid_destroy;
-
- s->btdev.device.class[0] = (class >> 0) & 0xff;
- s->btdev.device.class[1] = (class >> 8) & 0xff;
- s->btdev.device.class[2] = (class >> 16) & 0xff;
-
- return &s->btdev.device;
-}
-
-struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net)
-{
- return bt_hid_init(net, class_keyboard);
-}
diff --git a/vl.c b/vl.c
index 55bab005b6..2a12fc7f3e 100644
--- a/vl.c
+++ b/vl.c
@@ -988,37 +988,6 @@ static void bt_vhci_add(int vlan_id)
bt_vhci_init(bt_new_hci(vlan));
}
-static struct bt_device_s *bt_device_add(const char *opt)
-{
- struct bt_scatternet_s *vlan;
- int vlan_id = 0;
- char *endp = strstr(opt, ",vlan=");
- int len = (endp ? endp - opt : strlen(opt)) + 1;
- char devname[10];
-
- pstrcpy(devname, MIN(sizeof(devname), len), opt);
-
- if (endp) {
- vlan_id = strtol(endp + 6, &endp, 0);
- if (*endp) {
- error_report("unrecognised bluetooth vlan Id");
- return 0;
- }
- }
-
- vlan = qemu_find_bt_vlan(vlan_id);
-
- if (!vlan->slave)
- warn_report("adding a slave device to an empty scatternet %i",
- vlan_id);
-
- if (!strcmp(devname, "keyboard"))
- return bt_keyboard_init(vlan);
-
- error_report("unsupported bluetooth device '%s'", devname);
- return 0;
-}
-
static int bt_parse(const char *opt)
{
const char *endp, *p;
@@ -1051,8 +1020,7 @@ static int bt_parse(const char *opt)
bt_vhci_add(vlan);
return 0;
}
- } else if (strstart(opt, "device:", &endp))
- return !bt_device_add(endp);
+ }
error_report("bad bluetooth parameter '%s'", opt);
return 1;
diff --git a/hw/bt/Makefile.objs b/hw/bt/Makefile.objs
index 867a7d2e8a..a33983b522 100644
--- a/hw/bt/Makefile.objs
+++ b/hw/bt/Makefile.objs
@@ -1,3 +1,2 @@
-common-obj-y += core.o l2cap.o sdp.o hci.o hid.o
+common-obj-y += core.o l2cap.o sdp.o hci.o
common-obj-y += hci-csr.o
-
diff --git a/qemu-options.hx b/qemu-options.hx
index 38c7a978c1..48885cdca8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2804,15 +2804,6 @@ be used as following:
qemu-system-i386 [...OPTIONS...] -bt hci,vlan=5 -bt vhci,vlan=5
@end example
-@item -bt device:@var{dev}[,vlan=@var{n}]
-Emulate a bluetooth device @var{dev} and place it in network @var{n}
-(default @code{0}). QEMU can only emulate one type of bluetooth devices
-currently:
-
-@table @option
-@item keyboard
-Virtual wireless keyboard implementing the HIDP bluetooth profile.
-@end table
ETEXI
STEXI
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
2018-11-09 14:14 [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation Gerd Hoffmann
@ 2018-11-09 14:52 ` Liam Merwick
2018-11-09 16:36 ` Eric Blake
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Liam Merwick @ 2018-11-09 14:52 UTC (permalink / raw)
To: qemu-devel
On 09/11/2018 14:14, Gerd Hoffmann wrote:
> Broken (segfaultson first keypress) and appearently unused.
>
s/segfaultson/segfaults on/
s/appearently/apparently/
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
one question at the end, otherwise
Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
> ---
> include/hw/bt.h | 3 -
> hw/bt/hid.c | 554 ----------------------------------------------------
> vl.c | 34 +---
> hw/bt/Makefile.objs | 3 +-
> qemu-options.hx | 9 -
> 5 files changed, 2 insertions(+), 601 deletions(-)
> delete mode 100644 hw/bt/hid.c
>
> diff --git a/include/hw/bt.h b/include/hw/bt.h
> index b5e11d4d43..73fb1911f6 100644
> --- a/include/hw/bt.h
> +++ b/include/hw/bt.h
> @@ -173,9 +173,6 @@ enum bt_l2cap_psm_predef {
> /* bt-sdp.c */
> void bt_l2cap_sdp_init(struct bt_l2cap_device_s *dev);
>
> -/* bt-hid.c */
> -struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net);
> -
> /* Link Management Protocol layer defines */
>
> #define LLID_ACLU_CONT 0x1
> diff --git a/hw/bt/hid.c b/hw/bt/hid.c
> deleted file mode 100644
> index 056291f9b5..0000000000
> --- a/hw/bt/hid.c
> +++ /dev/null
> @@ -1,554 +0,0 @@
> -/*
> - * QEMU Bluetooth HID Profile wrapper for USB HID.
> - *
> - * Copyright (C) 2007-2008 OpenMoko, Inc.
> - * Written by Andrzej Zaborowski <andrew@openedhand.com>
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation; either version 2 or
> - * (at your option) version 3 of the License.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, if not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qemu-common.h"
> -#include "qemu/timer.h"
> -#include "ui/console.h"
> -#include "hw/input/hid.h"
> -#include "hw/bt.h"
> -
> -enum hid_transaction_req {
> - BT_HANDSHAKE = 0x0,
> - BT_HID_CONTROL = 0x1,
> - BT_GET_REPORT = 0x4,
> - BT_SET_REPORT = 0x5,
> - BT_GET_PROTOCOL = 0x6,
> - BT_SET_PROTOCOL = 0x7,
> - BT_GET_IDLE = 0x8,
> - BT_SET_IDLE = 0x9,
> - BT_DATA = 0xa,
> - BT_DATC = 0xb,
> -};
> -
> -enum hid_transaction_handshake {
> - BT_HS_SUCCESSFUL = 0x0,
> - BT_HS_NOT_READY = 0x1,
> - BT_HS_ERR_INVALID_REPORT_ID = 0x2,
> - BT_HS_ERR_UNSUPPORTED_REQUEST = 0x3,
> - BT_HS_ERR_INVALID_PARAMETER = 0x4,
> - BT_HS_ERR_UNKNOWN = 0xe,
> - BT_HS_ERR_FATAL = 0xf,
> -};
> -
> -enum hid_transaction_control {
> - BT_HC_NOP = 0x0,
> - BT_HC_HARD_RESET = 0x1,
> - BT_HC_SOFT_RESET = 0x2,
> - BT_HC_SUSPEND = 0x3,
> - BT_HC_EXIT_SUSPEND = 0x4,
> - BT_HC_VIRTUAL_CABLE_UNPLUG = 0x5,
> -};
> -
> -enum hid_protocol {
> - BT_HID_PROTO_BOOT = 0,
> - BT_HID_PROTO_REPORT = 1,
> -};
> -
> -enum hid_boot_reportid {
> - BT_HID_BOOT_INVALID = 0,
> - BT_HID_BOOT_KEYBOARD,
> - BT_HID_BOOT_MOUSE,
> -};
> -
> -enum hid_data_pkt {
> - BT_DATA_OTHER = 0,
> - BT_DATA_INPUT,
> - BT_DATA_OUTPUT,
> - BT_DATA_FEATURE,
> -};
> -
> -#define BT_HID_MTU 48
> -
> -/* HID interface requests */
> -#define GET_REPORT 0xa101
> -#define GET_IDLE 0xa102
> -#define GET_PROTOCOL 0xa103
> -#define SET_REPORT 0x2109
> -#define SET_IDLE 0x210a
> -#define SET_PROTOCOL 0x210b
> -
> -struct bt_hid_device_s {
> - struct bt_l2cap_device_s btdev;
> - struct bt_l2cap_conn_params_s *control;
> - struct bt_l2cap_conn_params_s *interrupt;
> - HIDState hid;
> -
> - int proto;
> - int connected;
> - int data_type;
> - int intr_state;
> - struct {
> - int len;
> - uint8_t buffer[1024];
> - } dataother, datain, dataout, feature, intrdataout;
> - enum {
> - bt_state_ready,
> - bt_state_transaction,
> - bt_state_suspend,
> - } state;
> -};
> -
> -static void bt_hid_reset(struct bt_hid_device_s *s)
> -{
> - struct bt_scatternet_s *net = s->btdev.device.net;
> -
> - /* Go as far as... */
> - bt_l2cap_device_done(&s->btdev);
> - bt_l2cap_device_init(&s->btdev, net);
> -
> - hid_reset(&s->hid);
> - s->proto = BT_HID_PROTO_REPORT;
> - s->state = bt_state_ready;
> - s->dataother.len = 0;
> - s->datain.len = 0;
> - s->dataout.len = 0;
> - s->feature.len = 0;
> - s->intrdataout.len = 0;
> - s->intr_state = 0;
> -}
> -
> -static int bt_hid_out(struct bt_hid_device_s *s)
> -{
> - if (s->data_type == BT_DATA_OUTPUT) {
> - /* nothing */
> - ;
> - }
> -
> - if (s->data_type == BT_DATA_FEATURE) {
> - /* XXX:
> - * does this send a USB_REQ_CLEAR_FEATURE/USB_REQ_SET_FEATURE
> - * or a SET_REPORT? */
> - ;
> - }
> -
> - return -1;
> -}
> -
> -static int bt_hid_in(struct bt_hid_device_s *s)
> -{
> - s->datain.len = hid_keyboard_poll(&s->hid, s->datain.buffer,
> - sizeof(s->datain.buffer));
> - return s->datain.len;
> -}
> -
> -static void bt_hid_send_handshake(struct bt_hid_device_s *s, int result)
> -{
> - *s->control->sdu_out(s->control, 1) =
> - (BT_HANDSHAKE << 4) | result;
> - s->control->sdu_submit(s->control);
> -}
> -
> -static void bt_hid_send_control(struct bt_hid_device_s *s, int operation)
> -{
> - *s->control->sdu_out(s->control, 1) =
> - (BT_HID_CONTROL << 4) | operation;
> - s->control->sdu_submit(s->control);
> -}
> -
> -static void bt_hid_disconnect(struct bt_hid_device_s *s)
> -{
> - /* Disconnect s->control and s->interrupt */
> -}
> -
> -static void bt_hid_send_data(struct bt_l2cap_conn_params_s *ch, int type,
> - const uint8_t *data, int len)
> -{
> - uint8_t *pkt, hdr = (BT_DATA << 4) | type;
> - int plen;
> -
> - do {
> - plen = MIN(len, ch->remote_mtu - 1);
> - pkt = ch->sdu_out(ch, plen + 1);
> -
> - pkt[0] = hdr;
> - if (plen)
> - memcpy(pkt + 1, data, plen);
> - ch->sdu_submit(ch);
> -
> - len -= plen;
> - data += plen;
> - hdr = (BT_DATC << 4) | type;
> - } while (plen == ch->remote_mtu - 1);
> -}
> -
> -static void bt_hid_control_transaction(struct bt_hid_device_s *s,
> - const uint8_t *data, int len)
> -{
> - uint8_t type, parameter;
> - int rlen, ret = -1;
> - if (len < 1)
> - return;
> -
> - type = data[0] >> 4;
> - parameter = data[0] & 0xf;
> -
> - switch (type) {
> - case BT_HANDSHAKE:
> - case BT_DATA:
> - switch (parameter) {
> - default:
> - /* These are not expected to be sent this direction. */
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - }
> - break;
> -
> - case BT_HID_CONTROL:
> - if (len != 1 || (parameter != BT_HC_VIRTUAL_CABLE_UNPLUG &&
> - s->state == bt_state_transaction)) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - switch (parameter) {
> - case BT_HC_NOP:
> - break;
> - case BT_HC_HARD_RESET:
> - case BT_HC_SOFT_RESET:
> - bt_hid_reset(s);
> - break;
> - case BT_HC_SUSPEND:
> - if (s->state == bt_state_ready)
> - s->state = bt_state_suspend;
> - else
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - case BT_HC_EXIT_SUSPEND:
> - if (s->state == bt_state_suspend)
> - s->state = bt_state_ready;
> - else
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - case BT_HC_VIRTUAL_CABLE_UNPLUG:
> - bt_hid_disconnect(s);
> - break;
> - default:
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - }
> - break;
> -
> - case BT_GET_REPORT:
> - /* No ReportIDs declared. */
> - if (((parameter & 8) && len != 3) ||
> - (!(parameter & 8) && len != 1) ||
> - s->state != bt_state_ready) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - if (parameter & 8)
> - rlen = data[2] | (data[3] << 8);
> - else
> - rlen = INT_MAX;
> - switch (parameter & 3) {
> - case BT_DATA_OTHER:
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - case BT_DATA_INPUT:
> - /* Here we can as well poll s->usbdev */
> - bt_hid_send_data(s->control, BT_DATA_INPUT,
> - s->datain.buffer, MIN(rlen, s->datain.len));
> - break;
> - case BT_DATA_OUTPUT:
> - bt_hid_send_data(s->control, BT_DATA_OUTPUT,
> - s->dataout.buffer, MIN(rlen, s->dataout.len));
> - break;
> - case BT_DATA_FEATURE:
> - bt_hid_send_data(s->control, BT_DATA_FEATURE,
> - s->feature.buffer, MIN(rlen, s->feature.len));
> - break;
> - }
> - break;
> -
> - case BT_SET_REPORT:
> - if (len < 2 || len > BT_HID_MTU || s->state != bt_state_ready ||
> - (parameter & 3) == BT_DATA_OTHER ||
> - (parameter & 3) == BT_DATA_INPUT) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - s->data_type = parameter & 3;
> - if (s->data_type == BT_DATA_OUTPUT) {
> - s->dataout.len = len - 1;
> - memcpy(s->dataout.buffer, data + 1, s->dataout.len);
> - } else {
> - s->feature.len = len - 1;
> - memcpy(s->feature.buffer, data + 1, s->feature.len);
> - }
> - if (len == BT_HID_MTU)
> - s->state = bt_state_transaction;
> - else
> - bt_hid_out(s);
> - break;
> -
> - case BT_GET_PROTOCOL:
> - if (len != 1 || s->state == bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - *s->control->sdu_out(s->control, 1) = s->proto;
> - s->control->sdu_submit(s->control);
> - break;
> -
> - case BT_SET_PROTOCOL:
> - if (len != 1 || s->state == bt_state_transaction ||
> - (parameter != BT_HID_PROTO_BOOT &&
> - parameter != BT_HID_PROTO_REPORT)) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - s->proto = parameter;
> - s->hid.protocol = parameter;
> - ret = BT_HS_SUCCESSFUL;
> - break;
> -
> - case BT_GET_IDLE:
> - if (len != 1 || s->state == bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - *s->control->sdu_out(s->control, 1) = s->hid.idle;
> - s->control->sdu_submit(s->control);
> - break;
> -
> - case BT_SET_IDLE:
> - if (len != 2 || s->state == bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> -
> - s->hid.idle = data[1];
> - /* XXX: Does this generate a handshake? */
> - break;
> -
> - case BT_DATC:
> - if (len > BT_HID_MTU || s->state != bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - if (s->data_type == BT_DATA_OUTPUT) {
> - memcpy(s->dataout.buffer + s->dataout.len, data + 1, len - 1);
> - s->dataout.len += len - 1;
> - } else {
> - memcpy(s->feature.buffer + s->feature.len, data + 1, len - 1);
> - s->feature.len += len - 1;
> - }
> - if (len < BT_HID_MTU) {
> - bt_hid_out(s);
> - s->state = bt_state_ready;
> - }
> - break;
> -
> - default:
> - ret = BT_HS_ERR_UNSUPPORTED_REQUEST;
> - }
> -
> - if (ret != -1)
> - bt_hid_send_handshake(s, ret);
> -}
> -
> -static void bt_hid_control_sdu(void *opaque, const uint8_t *data, int len)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - bt_hid_control_transaction(hid, data, len);
> -}
> -
> -static void bt_hid_datain(HIDState *hs)
> -{
> - struct bt_hid_device_s *hid =
> - container_of(hs, struct bt_hid_device_s, hid);
> -
> - /* If suspended, wake-up and send a wake-up event first. We might
> - * want to also inspect the input report and ignore event like
> - * mouse movements until a button event occurs. */
> - if (hid->state == bt_state_suspend) {
> - hid->state = bt_state_ready;
> - }
> -
> - if (bt_hid_in(hid) > 0)
> - /* TODO: when in boot-mode precede any Input reports with the ReportID
> - * byte, here and in GetReport/SetReport on the Control channel. */
> - bt_hid_send_data(hid->interrupt, BT_DATA_INPUT,
> - hid->datain.buffer, hid->datain.len);
> -}
> -
> -static void bt_hid_interrupt_sdu(void *opaque, const uint8_t *data, int len)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - if (len > BT_HID_MTU || len < 1)
> - goto bad;
> - if ((data[0] & 3) != BT_DATA_OUTPUT)
> - goto bad;
> - if ((data[0] >> 4) == BT_DATA) {
> - if (hid->intr_state)
> - goto bad;
> -
> - hid->data_type = BT_DATA_OUTPUT;
> - hid->intrdataout.len = 0;
> - } else if ((data[0] >> 4) == BT_DATC) {
> - if (!hid->intr_state)
> - goto bad;
> - } else
> - goto bad;
> -
> - memcpy(hid->intrdataout.buffer + hid->intrdataout.len, data + 1, len - 1);
> - hid->intrdataout.len += len - 1;
> - hid->intr_state = (len == BT_HID_MTU);
> - if (!hid->intr_state) {
> - memcpy(hid->dataout.buffer, hid->intrdataout.buffer,
> - hid->dataout.len = hid->intrdataout.len);
> - bt_hid_out(hid);
> - }
> -
> - return;
> -bad:
> - error_report("%s: bad transaction on Interrupt channel.",
> - __func__);
> -}
> -
> -/* "Virtual cable" plug/unplug event. */
> -static void bt_hid_connected_update(struct bt_hid_device_s *hid)
> -{
> - int prev = hid->connected;
> -
> - hid->connected = hid->control && hid->interrupt;
> -
> - /* Stop page-/inquiry-scanning when a host is connected. */
> - hid->btdev.device.page_scan = !hid->connected;
> - hid->btdev.device.inquiry_scan = !hid->connected;
> -
> - if (hid->connected && !prev) {
> - hid_reset(&hid->hid);
> - hid->proto = BT_HID_PROTO_REPORT;
> - }
> -
> - /* Should set HIDVirtualCable in SDP (possibly need to check that SDP
> - * isn't destroyed yet, in case we're being called from handle_destroy) */
> -}
> -
> -static void bt_hid_close_control(void *opaque)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - hid->control = NULL;
> - bt_hid_connected_update(hid);
> -}
> -
> -static void bt_hid_close_interrupt(void *opaque)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - hid->interrupt = NULL;
> - bt_hid_connected_update(hid);
> -}
> -
> -static int bt_hid_new_control_ch(struct bt_l2cap_device_s *dev,
> - struct bt_l2cap_conn_params_s *params)
> -{
> - struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
> -
> - if (hid->control)
> - return 1;
> -
> - hid->control = params;
> - hid->control->opaque = hid;
> - hid->control->close = bt_hid_close_control;
> - hid->control->sdu_in = bt_hid_control_sdu;
> -
> - bt_hid_connected_update(hid);
> -
> - return 0;
> -}
> -
> -static int bt_hid_new_interrupt_ch(struct bt_l2cap_device_s *dev,
> - struct bt_l2cap_conn_params_s *params)
> -{
> - struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
> -
> - if (hid->interrupt)
> - return 1;
> -
> - hid->interrupt = params;
> - hid->interrupt->opaque = hid;
> - hid->interrupt->close = bt_hid_close_interrupt;
> - hid->interrupt->sdu_in = bt_hid_interrupt_sdu;
> -
> - bt_hid_connected_update(hid);
> -
> - return 0;
> -}
> -
> -static void bt_hid_destroy(struct bt_device_s *dev)
> -{
> - struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
> -
> - if (hid->connected)
> - bt_hid_send_control(hid, BT_HC_VIRTUAL_CABLE_UNPLUG);
> - bt_l2cap_device_done(&hid->btdev);
> -
> - hid_free(&hid->hid);
> -
> - g_free(hid);
> -}
> -
> -enum peripheral_minor_class {
> - class_other = 0 << 4,
> - class_keyboard = 1 << 4,
> - class_pointing = 2 << 4,
> - class_combo = 3 << 4,
> -};
> -
> -static struct bt_device_s *bt_hid_init(struct bt_scatternet_s *net,
> - enum peripheral_minor_class minor)
> -{
> - struct bt_hid_device_s *s = g_malloc0(sizeof(*s));
> - uint32_t class =
> - /* Format type */
> - (0 << 0) |
> - /* Device class */
> - (minor << 2) |
> - (5 << 8) | /* "Peripheral" */
> - /* Service classes */
> - (1 << 13) | /* Limited discoverable mode */
> - (1 << 19); /* Capturing device (?) */
> -
> - bt_l2cap_device_init(&s->btdev, net);
> - bt_l2cap_sdp_init(&s->btdev);
> - bt_l2cap_psm_register(&s->btdev, BT_PSM_HID_CTRL,
> - BT_HID_MTU, bt_hid_new_control_ch);
> - bt_l2cap_psm_register(&s->btdev, BT_PSM_HID_INTR,
> - BT_HID_MTU, bt_hid_new_interrupt_ch);
> -
> - hid_init(&s->hid, HID_KEYBOARD, bt_hid_datain);
> - s->btdev.device.lmp_name = "BT Keyboard";
> -
> - s->btdev.device.handle_destroy = bt_hid_destroy;
> -
> - s->btdev.device.class[0] = (class >> 0) & 0xff;
> - s->btdev.device.class[1] = (class >> 8) & 0xff;
> - s->btdev.device.class[2] = (class >> 16) & 0xff;
> -
> - return &s->btdev.device;
> -}
> -
> -struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net)
> -{
> - return bt_hid_init(net, class_keyboard);
> -}
> diff --git a/vl.c b/vl.c
> index 55bab005b6..2a12fc7f3e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -988,37 +988,6 @@ static void bt_vhci_add(int vlan_id)
> bt_vhci_init(bt_new_hci(vlan));
> }
>
> -static struct bt_device_s *bt_device_add(const char *opt)
> -{
> - struct bt_scatternet_s *vlan;
> - int vlan_id = 0;
> - char *endp = strstr(opt, ",vlan=");
> - int len = (endp ? endp - opt : strlen(opt)) + 1;
> - char devname[10];
> -
> - pstrcpy(devname, MIN(sizeof(devname), len), opt);
> -
> - if (endp) {
> - vlan_id = strtol(endp + 6, &endp, 0);
> - if (*endp) {
> - error_report("unrecognised bluetooth vlan Id");
> - return 0;
> - }
> - }
> -
> - vlan = qemu_find_bt_vlan(vlan_id);
> -
> - if (!vlan->slave)
> - warn_report("adding a slave device to an empty scatternet %i",
> - vlan_id);
> -
> - if (!strcmp(devname, "keyboard"))
> - return bt_keyboard_init(vlan);
> -
> - error_report("unsupported bluetooth device '%s'", devname);
> - return 0;
> -}
> -
> static int bt_parse(const char *opt)
> {
> const char *endp, *p;
> @@ -1051,8 +1020,7 @@ static int bt_parse(const char *opt)
> bt_vhci_add(vlan);
> return 0;
> }
> - } else if (strstart(opt, "device:", &endp))
> - return !bt_device_add(endp);
> + }
>
> error_report("bad bluetooth parameter '%s'", opt);
> return 1;
> diff --git a/hw/bt/Makefile.objs b/hw/bt/Makefile.objs
> index 867a7d2e8a..a33983b522 100644
> --- a/hw/bt/Makefile.objs
> +++ b/hw/bt/Makefile.objs
> @@ -1,3 +1,2 @@
> -common-obj-y += core.o l2cap.o sdp.o hci.o hid.o
> +common-obj-y += core.o l2cap.o sdp.o hci.o
> common-obj-y += hci-csr.o
> -
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 38c7a978c1..48885cdca8 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
In this file, should these two lines also be deleted?
2761 "-bt device:dev[,vlan=n]\n" \
2762 " emulate a bluetooth device 'dev' in scatternet
'n'\n",
> @@ -2804,15 +2804,6 @@ be used as following:
> qemu-system-i386 [...OPTIONS...] -bt hci,vlan=5 -bt vhci,vlan=5
> @end example
>
> -@item -bt device:@var{dev}[,vlan=@var{n}]
> -Emulate a bluetooth device @var{dev} and place it in network @var{n}
> -(default @code{0}). QEMU can only emulate one type of bluetooth devices
> -currently:
> -
> -@table @option
> -@item keyboard
> -Virtual wireless keyboard implementing the HIDP bluetooth profile.
> -@end table
> ETEXI
>
> STEXI
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
2018-11-09 14:14 [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation Gerd Hoffmann
2018-11-09 14:52 ` Liam Merwick
@ 2018-11-09 16:36 ` Eric Blake
2018-11-09 19:02 ` Peter Maydell
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2018-11-09 16:36 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Paolo Bonzini
On 11/9/18 8:14 AM, Gerd Hoffmann wrote:
> Broken (segfaultson first keypress) and appearently unused.
s/segfaultson/segfaults on/, s/appearently/apparently/
Also, trailing '.' in the subject line is unusual
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
2018-11-09 14:14 [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation Gerd Hoffmann
2018-11-09 14:52 ` Liam Merwick
2018-11-09 16:36 ` Eric Blake
@ 2018-11-09 19:02 ` Peter Maydell
2018-11-12 9:16 ` Markus Armbruster
2018-11-09 19:36 ` Philippe Mathieu-Daudé
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2018-11-09 19:02 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: QEMU Developers, Paolo Bonzini
On 9 November 2018 at 14:14, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Broken (segfaultson first keypress) and appearently unused.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> include/hw/bt.h | 3 -
> hw/bt/hid.c | 554 ----------------------------------------------------
> vl.c | 34 +---
> hw/bt/Makefile.objs | 3 +-
> qemu-options.hx | 9 -
> 5 files changed, 2 insertions(+), 601 deletions(-)
> delete mode 100644 hw/bt/hid.c
Are we definitely happy that all the use cases for
this code segfault, not just the one you tested ?
Does it cost us much to mark this deprecated in 3.1
and drop it in 4.0 ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
2018-11-09 14:14 [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation Gerd Hoffmann
` (2 preceding siblings ...)
2018-11-09 19:02 ` Peter Maydell
@ 2018-11-09 19:36 ` Philippe Mathieu-Daudé
2018-11-09 21:11 ` Paolo Bonzini
2018-11-12 10:16 ` Thomas Huth
5 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-09 19:36 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel, Andrzej Zaborowski; +Cc: Paolo Bonzini
Cc'ing Andrzej, the actual author.
On 9/11/18 15:14, Gerd Hoffmann wrote:
> Broken (segfaultson first keypress) and appearently unused.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> include/hw/bt.h | 3 -
> hw/bt/hid.c | 554 ----------------------------------------------------
> vl.c | 34 +---
> hw/bt/Makefile.objs | 3 +-
> qemu-options.hx | 9 -
> 5 files changed, 2 insertions(+), 601 deletions(-)
> delete mode 100644 hw/bt/hid.c
>
> diff --git a/include/hw/bt.h b/include/hw/bt.h
> index b5e11d4d43..73fb1911f6 100644
> --- a/include/hw/bt.h
> +++ b/include/hw/bt.h
> @@ -173,9 +173,6 @@ enum bt_l2cap_psm_predef {
> /* bt-sdp.c */
> void bt_l2cap_sdp_init(struct bt_l2cap_device_s *dev);
>
> -/* bt-hid.c */
> -struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net);
> -
> /* Link Management Protocol layer defines */
>
> #define LLID_ACLU_CONT 0x1
> diff --git a/hw/bt/hid.c b/hw/bt/hid.c
> deleted file mode 100644
> index 056291f9b5..0000000000
> --- a/hw/bt/hid.c
> +++ /dev/null
> @@ -1,554 +0,0 @@
> -/*
> - * QEMU Bluetooth HID Profile wrapper for USB HID.
> - *
> - * Copyright (C) 2007-2008 OpenMoko, Inc.
> - * Written by Andrzej Zaborowski <andrew@openedhand.com>
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation; either version 2 or
> - * (at your option) version 3 of the License.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, if not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qemu-common.h"
> -#include "qemu/timer.h"
> -#include "ui/console.h"
> -#include "hw/input/hid.h"
> -#include "hw/bt.h"
> -
> -enum hid_transaction_req {
> - BT_HANDSHAKE = 0x0,
> - BT_HID_CONTROL = 0x1,
> - BT_GET_REPORT = 0x4,
> - BT_SET_REPORT = 0x5,
> - BT_GET_PROTOCOL = 0x6,
> - BT_SET_PROTOCOL = 0x7,
> - BT_GET_IDLE = 0x8,
> - BT_SET_IDLE = 0x9,
> - BT_DATA = 0xa,
> - BT_DATC = 0xb,
> -};
> -
> -enum hid_transaction_handshake {
> - BT_HS_SUCCESSFUL = 0x0,
> - BT_HS_NOT_READY = 0x1,
> - BT_HS_ERR_INVALID_REPORT_ID = 0x2,
> - BT_HS_ERR_UNSUPPORTED_REQUEST = 0x3,
> - BT_HS_ERR_INVALID_PARAMETER = 0x4,
> - BT_HS_ERR_UNKNOWN = 0xe,
> - BT_HS_ERR_FATAL = 0xf,
> -};
> -
> -enum hid_transaction_control {
> - BT_HC_NOP = 0x0,
> - BT_HC_HARD_RESET = 0x1,
> - BT_HC_SOFT_RESET = 0x2,
> - BT_HC_SUSPEND = 0x3,
> - BT_HC_EXIT_SUSPEND = 0x4,
> - BT_HC_VIRTUAL_CABLE_UNPLUG = 0x5,
> -};
> -
> -enum hid_protocol {
> - BT_HID_PROTO_BOOT = 0,
> - BT_HID_PROTO_REPORT = 1,
> -};
> -
> -enum hid_boot_reportid {
> - BT_HID_BOOT_INVALID = 0,
> - BT_HID_BOOT_KEYBOARD,
> - BT_HID_BOOT_MOUSE,
> -};
> -
> -enum hid_data_pkt {
> - BT_DATA_OTHER = 0,
> - BT_DATA_INPUT,
> - BT_DATA_OUTPUT,
> - BT_DATA_FEATURE,
> -};
> -
> -#define BT_HID_MTU 48
> -
> -/* HID interface requests */
> -#define GET_REPORT 0xa101
> -#define GET_IDLE 0xa102
> -#define GET_PROTOCOL 0xa103
> -#define SET_REPORT 0x2109
> -#define SET_IDLE 0x210a
> -#define SET_PROTOCOL 0x210b
> -
> -struct bt_hid_device_s {
> - struct bt_l2cap_device_s btdev;
> - struct bt_l2cap_conn_params_s *control;
> - struct bt_l2cap_conn_params_s *interrupt;
> - HIDState hid;
> -
> - int proto;
> - int connected;
> - int data_type;
> - int intr_state;
> - struct {
> - int len;
> - uint8_t buffer[1024];
> - } dataother, datain, dataout, feature, intrdataout;
> - enum {
> - bt_state_ready,
> - bt_state_transaction,
> - bt_state_suspend,
> - } state;
> -};
> -
> -static void bt_hid_reset(struct bt_hid_device_s *s)
> -{
> - struct bt_scatternet_s *net = s->btdev.device.net;
> -
> - /* Go as far as... */
> - bt_l2cap_device_done(&s->btdev);
> - bt_l2cap_device_init(&s->btdev, net);
> -
> - hid_reset(&s->hid);
> - s->proto = BT_HID_PROTO_REPORT;
> - s->state = bt_state_ready;
> - s->dataother.len = 0;
> - s->datain.len = 0;
> - s->dataout.len = 0;
> - s->feature.len = 0;
> - s->intrdataout.len = 0;
> - s->intr_state = 0;
> -}
> -
> -static int bt_hid_out(struct bt_hid_device_s *s)
> -{
> - if (s->data_type == BT_DATA_OUTPUT) {
> - /* nothing */
> - ;
> - }
> -
> - if (s->data_type == BT_DATA_FEATURE) {
> - /* XXX:
> - * does this send a USB_REQ_CLEAR_FEATURE/USB_REQ_SET_FEATURE
> - * or a SET_REPORT? */
> - ;
> - }
> -
> - return -1;
> -}
> -
> -static int bt_hid_in(struct bt_hid_device_s *s)
> -{
> - s->datain.len = hid_keyboard_poll(&s->hid, s->datain.buffer,
> - sizeof(s->datain.buffer));
> - return s->datain.len;
> -}
> -
> -static void bt_hid_send_handshake(struct bt_hid_device_s *s, int result)
> -{
> - *s->control->sdu_out(s->control, 1) =
> - (BT_HANDSHAKE << 4) | result;
> - s->control->sdu_submit(s->control);
> -}
> -
> -static void bt_hid_send_control(struct bt_hid_device_s *s, int operation)
> -{
> - *s->control->sdu_out(s->control, 1) =
> - (BT_HID_CONTROL << 4) | operation;
> - s->control->sdu_submit(s->control);
> -}
> -
> -static void bt_hid_disconnect(struct bt_hid_device_s *s)
> -{
> - /* Disconnect s->control and s->interrupt */
> -}
> -
> -static void bt_hid_send_data(struct bt_l2cap_conn_params_s *ch, int type,
> - const uint8_t *data, int len)
> -{
> - uint8_t *pkt, hdr = (BT_DATA << 4) | type;
> - int plen;
> -
> - do {
> - plen = MIN(len, ch->remote_mtu - 1);
> - pkt = ch->sdu_out(ch, plen + 1);
> -
> - pkt[0] = hdr;
> - if (plen)
> - memcpy(pkt + 1, data, plen);
> - ch->sdu_submit(ch);
> -
> - len -= plen;
> - data += plen;
> - hdr = (BT_DATC << 4) | type;
> - } while (plen == ch->remote_mtu - 1);
> -}
> -
> -static void bt_hid_control_transaction(struct bt_hid_device_s *s,
> - const uint8_t *data, int len)
> -{
> - uint8_t type, parameter;
> - int rlen, ret = -1;
> - if (len < 1)
> - return;
> -
> - type = data[0] >> 4;
> - parameter = data[0] & 0xf;
> -
> - switch (type) {
> - case BT_HANDSHAKE:
> - case BT_DATA:
> - switch (parameter) {
> - default:
> - /* These are not expected to be sent this direction. */
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - }
> - break;
> -
> - case BT_HID_CONTROL:
> - if (len != 1 || (parameter != BT_HC_VIRTUAL_CABLE_UNPLUG &&
> - s->state == bt_state_transaction)) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - switch (parameter) {
> - case BT_HC_NOP:
> - break;
> - case BT_HC_HARD_RESET:
> - case BT_HC_SOFT_RESET:
> - bt_hid_reset(s);
> - break;
> - case BT_HC_SUSPEND:
> - if (s->state == bt_state_ready)
> - s->state = bt_state_suspend;
> - else
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - case BT_HC_EXIT_SUSPEND:
> - if (s->state == bt_state_suspend)
> - s->state = bt_state_ready;
> - else
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - case BT_HC_VIRTUAL_CABLE_UNPLUG:
> - bt_hid_disconnect(s);
> - break;
> - default:
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - }
> - break;
> -
> - case BT_GET_REPORT:
> - /* No ReportIDs declared. */
> - if (((parameter & 8) && len != 3) ||
> - (!(parameter & 8) && len != 1) ||
> - s->state != bt_state_ready) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - if (parameter & 8)
> - rlen = data[2] | (data[3] << 8);
> - else
> - rlen = INT_MAX;
> - switch (parameter & 3) {
> - case BT_DATA_OTHER:
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - case BT_DATA_INPUT:
> - /* Here we can as well poll s->usbdev */
> - bt_hid_send_data(s->control, BT_DATA_INPUT,
> - s->datain.buffer, MIN(rlen, s->datain.len));
> - break;
> - case BT_DATA_OUTPUT:
> - bt_hid_send_data(s->control, BT_DATA_OUTPUT,
> - s->dataout.buffer, MIN(rlen, s->dataout.len));
> - break;
> - case BT_DATA_FEATURE:
> - bt_hid_send_data(s->control, BT_DATA_FEATURE,
> - s->feature.buffer, MIN(rlen, s->feature.len));
> - break;
> - }
> - break;
> -
> - case BT_SET_REPORT:
> - if (len < 2 || len > BT_HID_MTU || s->state != bt_state_ready ||
> - (parameter & 3) == BT_DATA_OTHER ||
> - (parameter & 3) == BT_DATA_INPUT) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - s->data_type = parameter & 3;
> - if (s->data_type == BT_DATA_OUTPUT) {
> - s->dataout.len = len - 1;
> - memcpy(s->dataout.buffer, data + 1, s->dataout.len);
> - } else {
> - s->feature.len = len - 1;
> - memcpy(s->feature.buffer, data + 1, s->feature.len);
> - }
> - if (len == BT_HID_MTU)
> - s->state = bt_state_transaction;
> - else
> - bt_hid_out(s);
> - break;
> -
> - case BT_GET_PROTOCOL:
> - if (len != 1 || s->state == bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - *s->control->sdu_out(s->control, 1) = s->proto;
> - s->control->sdu_submit(s->control);
> - break;
> -
> - case BT_SET_PROTOCOL:
> - if (len != 1 || s->state == bt_state_transaction ||
> - (parameter != BT_HID_PROTO_BOOT &&
> - parameter != BT_HID_PROTO_REPORT)) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - s->proto = parameter;
> - s->hid.protocol = parameter;
> - ret = BT_HS_SUCCESSFUL;
> - break;
> -
> - case BT_GET_IDLE:
> - if (len != 1 || s->state == bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - *s->control->sdu_out(s->control, 1) = s->hid.idle;
> - s->control->sdu_submit(s->control);
> - break;
> -
> - case BT_SET_IDLE:
> - if (len != 2 || s->state == bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> -
> - s->hid.idle = data[1];
> - /* XXX: Does this generate a handshake? */
> - break;
> -
> - case BT_DATC:
> - if (len > BT_HID_MTU || s->state != bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - if (s->data_type == BT_DATA_OUTPUT) {
> - memcpy(s->dataout.buffer + s->dataout.len, data + 1, len - 1);
> - s->dataout.len += len - 1;
> - } else {
> - memcpy(s->feature.buffer + s->feature.len, data + 1, len - 1);
> - s->feature.len += len - 1;
> - }
> - if (len < BT_HID_MTU) {
> - bt_hid_out(s);
> - s->state = bt_state_ready;
> - }
> - break;
> -
> - default:
> - ret = BT_HS_ERR_UNSUPPORTED_REQUEST;
> - }
> -
> - if (ret != -1)
> - bt_hid_send_handshake(s, ret);
> -}
> -
> -static void bt_hid_control_sdu(void *opaque, const uint8_t *data, int len)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - bt_hid_control_transaction(hid, data, len);
> -}
> -
> -static void bt_hid_datain(HIDState *hs)
> -{
> - struct bt_hid_device_s *hid =
> - container_of(hs, struct bt_hid_device_s, hid);
> -
> - /* If suspended, wake-up and send a wake-up event first. We might
> - * want to also inspect the input report and ignore event like
> - * mouse movements until a button event occurs. */
> - if (hid->state == bt_state_suspend) {
> - hid->state = bt_state_ready;
> - }
> -
> - if (bt_hid_in(hid) > 0)
> - /* TODO: when in boot-mode precede any Input reports with the ReportID
> - * byte, here and in GetReport/SetReport on the Control channel. */
> - bt_hid_send_data(hid->interrupt, BT_DATA_INPUT,
> - hid->datain.buffer, hid->datain.len);
> -}
> -
> -static void bt_hid_interrupt_sdu(void *opaque, const uint8_t *data, int len)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - if (len > BT_HID_MTU || len < 1)
> - goto bad;
> - if ((data[0] & 3) != BT_DATA_OUTPUT)
> - goto bad;
> - if ((data[0] >> 4) == BT_DATA) {
> - if (hid->intr_state)
> - goto bad;
> -
> - hid->data_type = BT_DATA_OUTPUT;
> - hid->intrdataout.len = 0;
> - } else if ((data[0] >> 4) == BT_DATC) {
> - if (!hid->intr_state)
> - goto bad;
> - } else
> - goto bad;
> -
> - memcpy(hid->intrdataout.buffer + hid->intrdataout.len, data + 1, len - 1);
> - hid->intrdataout.len += len - 1;
> - hid->intr_state = (len == BT_HID_MTU);
> - if (!hid->intr_state) {
> - memcpy(hid->dataout.buffer, hid->intrdataout.buffer,
> - hid->dataout.len = hid->intrdataout.len);
> - bt_hid_out(hid);
> - }
> -
> - return;
> -bad:
> - error_report("%s: bad transaction on Interrupt channel.",
> - __func__);
> -}
> -
> -/* "Virtual cable" plug/unplug event. */
> -static void bt_hid_connected_update(struct bt_hid_device_s *hid)
> -{
> - int prev = hid->connected;
> -
> - hid->connected = hid->control && hid->interrupt;
> -
> - /* Stop page-/inquiry-scanning when a host is connected. */
> - hid->btdev.device.page_scan = !hid->connected;
> - hid->btdev.device.inquiry_scan = !hid->connected;
> -
> - if (hid->connected && !prev) {
> - hid_reset(&hid->hid);
> - hid->proto = BT_HID_PROTO_REPORT;
> - }
> -
> - /* Should set HIDVirtualCable in SDP (possibly need to check that SDP
> - * isn't destroyed yet, in case we're being called from handle_destroy) */
> -}
> -
> -static void bt_hid_close_control(void *opaque)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - hid->control = NULL;
> - bt_hid_connected_update(hid);
> -}
> -
> -static void bt_hid_close_interrupt(void *opaque)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - hid->interrupt = NULL;
> - bt_hid_connected_update(hid);
> -}
> -
> -static int bt_hid_new_control_ch(struct bt_l2cap_device_s *dev,
> - struct bt_l2cap_conn_params_s *params)
> -{
> - struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
> -
> - if (hid->control)
> - return 1;
> -
> - hid->control = params;
> - hid->control->opaque = hid;
> - hid->control->close = bt_hid_close_control;
> - hid->control->sdu_in = bt_hid_control_sdu;
> -
> - bt_hid_connected_update(hid);
> -
> - return 0;
> -}
> -
> -static int bt_hid_new_interrupt_ch(struct bt_l2cap_device_s *dev,
> - struct bt_l2cap_conn_params_s *params)
> -{
> - struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
> -
> - if (hid->interrupt)
> - return 1;
> -
> - hid->interrupt = params;
> - hid->interrupt->opaque = hid;
> - hid->interrupt->close = bt_hid_close_interrupt;
> - hid->interrupt->sdu_in = bt_hid_interrupt_sdu;
> -
> - bt_hid_connected_update(hid);
> -
> - return 0;
> -}
> -
> -static void bt_hid_destroy(struct bt_device_s *dev)
> -{
> - struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
> -
> - if (hid->connected)
> - bt_hid_send_control(hid, BT_HC_VIRTUAL_CABLE_UNPLUG);
> - bt_l2cap_device_done(&hid->btdev);
> -
> - hid_free(&hid->hid);
> -
> - g_free(hid);
> -}
> -
> -enum peripheral_minor_class {
> - class_other = 0 << 4,
> - class_keyboard = 1 << 4,
> - class_pointing = 2 << 4,
> - class_combo = 3 << 4,
> -};
> -
> -static struct bt_device_s *bt_hid_init(struct bt_scatternet_s *net,
> - enum peripheral_minor_class minor)
> -{
> - struct bt_hid_device_s *s = g_malloc0(sizeof(*s));
> - uint32_t class =
> - /* Format type */
> - (0 << 0) |
> - /* Device class */
> - (minor << 2) |
> - (5 << 8) | /* "Peripheral" */
> - /* Service classes */
> - (1 << 13) | /* Limited discoverable mode */
> - (1 << 19); /* Capturing device (?) */
> -
> - bt_l2cap_device_init(&s->btdev, net);
> - bt_l2cap_sdp_init(&s->btdev);
> - bt_l2cap_psm_register(&s->btdev, BT_PSM_HID_CTRL,
> - BT_HID_MTU, bt_hid_new_control_ch);
> - bt_l2cap_psm_register(&s->btdev, BT_PSM_HID_INTR,
> - BT_HID_MTU, bt_hid_new_interrupt_ch);
> -
> - hid_init(&s->hid, HID_KEYBOARD, bt_hid_datain);
> - s->btdev.device.lmp_name = "BT Keyboard";
> -
> - s->btdev.device.handle_destroy = bt_hid_destroy;
> -
> - s->btdev.device.class[0] = (class >> 0) & 0xff;
> - s->btdev.device.class[1] = (class >> 8) & 0xff;
> - s->btdev.device.class[2] = (class >> 16) & 0xff;
> -
> - return &s->btdev.device;
> -}
> -
> -struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net)
> -{
> - return bt_hid_init(net, class_keyboard);
> -}
> diff --git a/vl.c b/vl.c
> index 55bab005b6..2a12fc7f3e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -988,37 +988,6 @@ static void bt_vhci_add(int vlan_id)
> bt_vhci_init(bt_new_hci(vlan));
> }
>
> -static struct bt_device_s *bt_device_add(const char *opt)
> -{
> - struct bt_scatternet_s *vlan;
> - int vlan_id = 0;
> - char *endp = strstr(opt, ",vlan=");
> - int len = (endp ? endp - opt : strlen(opt)) + 1;
> - char devname[10];
> -
> - pstrcpy(devname, MIN(sizeof(devname), len), opt);
> -
> - if (endp) {
> - vlan_id = strtol(endp + 6, &endp, 0);
> - if (*endp) {
> - error_report("unrecognised bluetooth vlan Id");
> - return 0;
> - }
> - }
> -
> - vlan = qemu_find_bt_vlan(vlan_id);
> -
> - if (!vlan->slave)
> - warn_report("adding a slave device to an empty scatternet %i",
> - vlan_id);
> -
> - if (!strcmp(devname, "keyboard"))
> - return bt_keyboard_init(vlan);
> -
> - error_report("unsupported bluetooth device '%s'", devname);
> - return 0;
> -}
> -
> static int bt_parse(const char *opt)
> {
> const char *endp, *p;
> @@ -1051,8 +1020,7 @@ static int bt_parse(const char *opt)
> bt_vhci_add(vlan);
> return 0;
> }
> - } else if (strstart(opt, "device:", &endp))
> - return !bt_device_add(endp);
> + }
>
> error_report("bad bluetooth parameter '%s'", opt);
> return 1;
> diff --git a/hw/bt/Makefile.objs b/hw/bt/Makefile.objs
> index 867a7d2e8a..a33983b522 100644
> --- a/hw/bt/Makefile.objs
> +++ b/hw/bt/Makefile.objs
> @@ -1,3 +1,2 @@
> -common-obj-y += core.o l2cap.o sdp.o hci.o hid.o
> +common-obj-y += core.o l2cap.o sdp.o hci.o
> common-obj-y += hci-csr.o
> -
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 38c7a978c1..48885cdca8 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2804,15 +2804,6 @@ be used as following:
> qemu-system-i386 [...OPTIONS...] -bt hci,vlan=5 -bt vhci,vlan=5
> @end example
>
> -@item -bt device:@var{dev}[,vlan=@var{n}]
> -Emulate a bluetooth device @var{dev} and place it in network @var{n}
> -(default @code{0}). QEMU can only emulate one type of bluetooth devices
> -currently:
> -
> -@table @option
> -@item keyboard
> -Virtual wireless keyboard implementing the HIDP bluetooth profile.
> -@end table
> ETEXI
>
> STEXI
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
2018-11-09 14:14 [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation Gerd Hoffmann
` (3 preceding siblings ...)
2018-11-09 19:36 ` Philippe Mathieu-Daudé
@ 2018-11-09 21:11 ` Paolo Bonzini
2018-11-12 10:16 ` Thomas Huth
5 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-11-09 21:11 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
On 09/11/2018 15:14, Gerd Hoffmann wrote:
> Broken (segfaultson first keypress) and appearently unused.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
No objection, but can we make some effort of, at least, putting the
stack backtrace in the commit log?
Paolo
> ---
> include/hw/bt.h | 3 -
> hw/bt/hid.c | 554 ----------------------------------------------------
> vl.c | 34 +---
> hw/bt/Makefile.objs | 3 +-
> qemu-options.hx | 9 -
> 5 files changed, 2 insertions(+), 601 deletions(-)
> delete mode 100644 hw/bt/hid.c
>
> diff --git a/include/hw/bt.h b/include/hw/bt.h
> index b5e11d4d43..73fb1911f6 100644
> --- a/include/hw/bt.h
> +++ b/include/hw/bt.h
> @@ -173,9 +173,6 @@ enum bt_l2cap_psm_predef {
> /* bt-sdp.c */
> void bt_l2cap_sdp_init(struct bt_l2cap_device_s *dev);
>
> -/* bt-hid.c */
> -struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net);
> -
> /* Link Management Protocol layer defines */
>
> #define LLID_ACLU_CONT 0x1
> diff --git a/hw/bt/hid.c b/hw/bt/hid.c
> deleted file mode 100644
> index 056291f9b5..0000000000
> --- a/hw/bt/hid.c
> +++ /dev/null
> @@ -1,554 +0,0 @@
> -/*
> - * QEMU Bluetooth HID Profile wrapper for USB HID.
> - *
> - * Copyright (C) 2007-2008 OpenMoko, Inc.
> - * Written by Andrzej Zaborowski <andrew@openedhand.com>
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation; either version 2 or
> - * (at your option) version 3 of the License.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, if not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qemu-common.h"
> -#include "qemu/timer.h"
> -#include "ui/console.h"
> -#include "hw/input/hid.h"
> -#include "hw/bt.h"
> -
> -enum hid_transaction_req {
> - BT_HANDSHAKE = 0x0,
> - BT_HID_CONTROL = 0x1,
> - BT_GET_REPORT = 0x4,
> - BT_SET_REPORT = 0x5,
> - BT_GET_PROTOCOL = 0x6,
> - BT_SET_PROTOCOL = 0x7,
> - BT_GET_IDLE = 0x8,
> - BT_SET_IDLE = 0x9,
> - BT_DATA = 0xa,
> - BT_DATC = 0xb,
> -};
> -
> -enum hid_transaction_handshake {
> - BT_HS_SUCCESSFUL = 0x0,
> - BT_HS_NOT_READY = 0x1,
> - BT_HS_ERR_INVALID_REPORT_ID = 0x2,
> - BT_HS_ERR_UNSUPPORTED_REQUEST = 0x3,
> - BT_HS_ERR_INVALID_PARAMETER = 0x4,
> - BT_HS_ERR_UNKNOWN = 0xe,
> - BT_HS_ERR_FATAL = 0xf,
> -};
> -
> -enum hid_transaction_control {
> - BT_HC_NOP = 0x0,
> - BT_HC_HARD_RESET = 0x1,
> - BT_HC_SOFT_RESET = 0x2,
> - BT_HC_SUSPEND = 0x3,
> - BT_HC_EXIT_SUSPEND = 0x4,
> - BT_HC_VIRTUAL_CABLE_UNPLUG = 0x5,
> -};
> -
> -enum hid_protocol {
> - BT_HID_PROTO_BOOT = 0,
> - BT_HID_PROTO_REPORT = 1,
> -};
> -
> -enum hid_boot_reportid {
> - BT_HID_BOOT_INVALID = 0,
> - BT_HID_BOOT_KEYBOARD,
> - BT_HID_BOOT_MOUSE,
> -};
> -
> -enum hid_data_pkt {
> - BT_DATA_OTHER = 0,
> - BT_DATA_INPUT,
> - BT_DATA_OUTPUT,
> - BT_DATA_FEATURE,
> -};
> -
> -#define BT_HID_MTU 48
> -
> -/* HID interface requests */
> -#define GET_REPORT 0xa101
> -#define GET_IDLE 0xa102
> -#define GET_PROTOCOL 0xa103
> -#define SET_REPORT 0x2109
> -#define SET_IDLE 0x210a
> -#define SET_PROTOCOL 0x210b
> -
> -struct bt_hid_device_s {
> - struct bt_l2cap_device_s btdev;
> - struct bt_l2cap_conn_params_s *control;
> - struct bt_l2cap_conn_params_s *interrupt;
> - HIDState hid;
> -
> - int proto;
> - int connected;
> - int data_type;
> - int intr_state;
> - struct {
> - int len;
> - uint8_t buffer[1024];
> - } dataother, datain, dataout, feature, intrdataout;
> - enum {
> - bt_state_ready,
> - bt_state_transaction,
> - bt_state_suspend,
> - } state;
> -};
> -
> -static void bt_hid_reset(struct bt_hid_device_s *s)
> -{
> - struct bt_scatternet_s *net = s->btdev.device.net;
> -
> - /* Go as far as... */
> - bt_l2cap_device_done(&s->btdev);
> - bt_l2cap_device_init(&s->btdev, net);
> -
> - hid_reset(&s->hid);
> - s->proto = BT_HID_PROTO_REPORT;
> - s->state = bt_state_ready;
> - s->dataother.len = 0;
> - s->datain.len = 0;
> - s->dataout.len = 0;
> - s->feature.len = 0;
> - s->intrdataout.len = 0;
> - s->intr_state = 0;
> -}
> -
> -static int bt_hid_out(struct bt_hid_device_s *s)
> -{
> - if (s->data_type == BT_DATA_OUTPUT) {
> - /* nothing */
> - ;
> - }
> -
> - if (s->data_type == BT_DATA_FEATURE) {
> - /* XXX:
> - * does this send a USB_REQ_CLEAR_FEATURE/USB_REQ_SET_FEATURE
> - * or a SET_REPORT? */
> - ;
> - }
> -
> - return -1;
> -}
> -
> -static int bt_hid_in(struct bt_hid_device_s *s)
> -{
> - s->datain.len = hid_keyboard_poll(&s->hid, s->datain.buffer,
> - sizeof(s->datain.buffer));
> - return s->datain.len;
> -}
> -
> -static void bt_hid_send_handshake(struct bt_hid_device_s *s, int result)
> -{
> - *s->control->sdu_out(s->control, 1) =
> - (BT_HANDSHAKE << 4) | result;
> - s->control->sdu_submit(s->control);
> -}
> -
> -static void bt_hid_send_control(struct bt_hid_device_s *s, int operation)
> -{
> - *s->control->sdu_out(s->control, 1) =
> - (BT_HID_CONTROL << 4) | operation;
> - s->control->sdu_submit(s->control);
> -}
> -
> -static void bt_hid_disconnect(struct bt_hid_device_s *s)
> -{
> - /* Disconnect s->control and s->interrupt */
> -}
> -
> -static void bt_hid_send_data(struct bt_l2cap_conn_params_s *ch, int type,
> - const uint8_t *data, int len)
> -{
> - uint8_t *pkt, hdr = (BT_DATA << 4) | type;
> - int plen;
> -
> - do {
> - plen = MIN(len, ch->remote_mtu - 1);
> - pkt = ch->sdu_out(ch, plen + 1);
> -
> - pkt[0] = hdr;
> - if (plen)
> - memcpy(pkt + 1, data, plen);
> - ch->sdu_submit(ch);
> -
> - len -= plen;
> - data += plen;
> - hdr = (BT_DATC << 4) | type;
> - } while (plen == ch->remote_mtu - 1);
> -}
> -
> -static void bt_hid_control_transaction(struct bt_hid_device_s *s,
> - const uint8_t *data, int len)
> -{
> - uint8_t type, parameter;
> - int rlen, ret = -1;
> - if (len < 1)
> - return;
> -
> - type = data[0] >> 4;
> - parameter = data[0] & 0xf;
> -
> - switch (type) {
> - case BT_HANDSHAKE:
> - case BT_DATA:
> - switch (parameter) {
> - default:
> - /* These are not expected to be sent this direction. */
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - }
> - break;
> -
> - case BT_HID_CONTROL:
> - if (len != 1 || (parameter != BT_HC_VIRTUAL_CABLE_UNPLUG &&
> - s->state == bt_state_transaction)) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - switch (parameter) {
> - case BT_HC_NOP:
> - break;
> - case BT_HC_HARD_RESET:
> - case BT_HC_SOFT_RESET:
> - bt_hid_reset(s);
> - break;
> - case BT_HC_SUSPEND:
> - if (s->state == bt_state_ready)
> - s->state = bt_state_suspend;
> - else
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - case BT_HC_EXIT_SUSPEND:
> - if (s->state == bt_state_suspend)
> - s->state = bt_state_ready;
> - else
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - case BT_HC_VIRTUAL_CABLE_UNPLUG:
> - bt_hid_disconnect(s);
> - break;
> - default:
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - }
> - break;
> -
> - case BT_GET_REPORT:
> - /* No ReportIDs declared. */
> - if (((parameter & 8) && len != 3) ||
> - (!(parameter & 8) && len != 1) ||
> - s->state != bt_state_ready) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - if (parameter & 8)
> - rlen = data[2] | (data[3] << 8);
> - else
> - rlen = INT_MAX;
> - switch (parameter & 3) {
> - case BT_DATA_OTHER:
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - case BT_DATA_INPUT:
> - /* Here we can as well poll s->usbdev */
> - bt_hid_send_data(s->control, BT_DATA_INPUT,
> - s->datain.buffer, MIN(rlen, s->datain.len));
> - break;
> - case BT_DATA_OUTPUT:
> - bt_hid_send_data(s->control, BT_DATA_OUTPUT,
> - s->dataout.buffer, MIN(rlen, s->dataout.len));
> - break;
> - case BT_DATA_FEATURE:
> - bt_hid_send_data(s->control, BT_DATA_FEATURE,
> - s->feature.buffer, MIN(rlen, s->feature.len));
> - break;
> - }
> - break;
> -
> - case BT_SET_REPORT:
> - if (len < 2 || len > BT_HID_MTU || s->state != bt_state_ready ||
> - (parameter & 3) == BT_DATA_OTHER ||
> - (parameter & 3) == BT_DATA_INPUT) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - s->data_type = parameter & 3;
> - if (s->data_type == BT_DATA_OUTPUT) {
> - s->dataout.len = len - 1;
> - memcpy(s->dataout.buffer, data + 1, s->dataout.len);
> - } else {
> - s->feature.len = len - 1;
> - memcpy(s->feature.buffer, data + 1, s->feature.len);
> - }
> - if (len == BT_HID_MTU)
> - s->state = bt_state_transaction;
> - else
> - bt_hid_out(s);
> - break;
> -
> - case BT_GET_PROTOCOL:
> - if (len != 1 || s->state == bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - *s->control->sdu_out(s->control, 1) = s->proto;
> - s->control->sdu_submit(s->control);
> - break;
> -
> - case BT_SET_PROTOCOL:
> - if (len != 1 || s->state == bt_state_transaction ||
> - (parameter != BT_HID_PROTO_BOOT &&
> - parameter != BT_HID_PROTO_REPORT)) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - s->proto = parameter;
> - s->hid.protocol = parameter;
> - ret = BT_HS_SUCCESSFUL;
> - break;
> -
> - case BT_GET_IDLE:
> - if (len != 1 || s->state == bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - *s->control->sdu_out(s->control, 1) = s->hid.idle;
> - s->control->sdu_submit(s->control);
> - break;
> -
> - case BT_SET_IDLE:
> - if (len != 2 || s->state == bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> -
> - s->hid.idle = data[1];
> - /* XXX: Does this generate a handshake? */
> - break;
> -
> - case BT_DATC:
> - if (len > BT_HID_MTU || s->state != bt_state_transaction) {
> - ret = BT_HS_ERR_INVALID_PARAMETER;
> - break;
> - }
> - if (s->data_type == BT_DATA_OUTPUT) {
> - memcpy(s->dataout.buffer + s->dataout.len, data + 1, len - 1);
> - s->dataout.len += len - 1;
> - } else {
> - memcpy(s->feature.buffer + s->feature.len, data + 1, len - 1);
> - s->feature.len += len - 1;
> - }
> - if (len < BT_HID_MTU) {
> - bt_hid_out(s);
> - s->state = bt_state_ready;
> - }
> - break;
> -
> - default:
> - ret = BT_HS_ERR_UNSUPPORTED_REQUEST;
> - }
> -
> - if (ret != -1)
> - bt_hid_send_handshake(s, ret);
> -}
> -
> -static void bt_hid_control_sdu(void *opaque, const uint8_t *data, int len)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - bt_hid_control_transaction(hid, data, len);
> -}
> -
> -static void bt_hid_datain(HIDState *hs)
> -{
> - struct bt_hid_device_s *hid =
> - container_of(hs, struct bt_hid_device_s, hid);
> -
> - /* If suspended, wake-up and send a wake-up event first. We might
> - * want to also inspect the input report and ignore event like
> - * mouse movements until a button event occurs. */
> - if (hid->state == bt_state_suspend) {
> - hid->state = bt_state_ready;
> - }
> -
> - if (bt_hid_in(hid) > 0)
> - /* TODO: when in boot-mode precede any Input reports with the ReportID
> - * byte, here and in GetReport/SetReport on the Control channel. */
> - bt_hid_send_data(hid->interrupt, BT_DATA_INPUT,
> - hid->datain.buffer, hid->datain.len);
> -}
> -
> -static void bt_hid_interrupt_sdu(void *opaque, const uint8_t *data, int len)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - if (len > BT_HID_MTU || len < 1)
> - goto bad;
> - if ((data[0] & 3) != BT_DATA_OUTPUT)
> - goto bad;
> - if ((data[0] >> 4) == BT_DATA) {
> - if (hid->intr_state)
> - goto bad;
> -
> - hid->data_type = BT_DATA_OUTPUT;
> - hid->intrdataout.len = 0;
> - } else if ((data[0] >> 4) == BT_DATC) {
> - if (!hid->intr_state)
> - goto bad;
> - } else
> - goto bad;
> -
> - memcpy(hid->intrdataout.buffer + hid->intrdataout.len, data + 1, len - 1);
> - hid->intrdataout.len += len - 1;
> - hid->intr_state = (len == BT_HID_MTU);
> - if (!hid->intr_state) {
> - memcpy(hid->dataout.buffer, hid->intrdataout.buffer,
> - hid->dataout.len = hid->intrdataout.len);
> - bt_hid_out(hid);
> - }
> -
> - return;
> -bad:
> - error_report("%s: bad transaction on Interrupt channel.",
> - __func__);
> -}
> -
> -/* "Virtual cable" plug/unplug event. */
> -static void bt_hid_connected_update(struct bt_hid_device_s *hid)
> -{
> - int prev = hid->connected;
> -
> - hid->connected = hid->control && hid->interrupt;
> -
> - /* Stop page-/inquiry-scanning when a host is connected. */
> - hid->btdev.device.page_scan = !hid->connected;
> - hid->btdev.device.inquiry_scan = !hid->connected;
> -
> - if (hid->connected && !prev) {
> - hid_reset(&hid->hid);
> - hid->proto = BT_HID_PROTO_REPORT;
> - }
> -
> - /* Should set HIDVirtualCable in SDP (possibly need to check that SDP
> - * isn't destroyed yet, in case we're being called from handle_destroy) */
> -}
> -
> -static void bt_hid_close_control(void *opaque)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - hid->control = NULL;
> - bt_hid_connected_update(hid);
> -}
> -
> -static void bt_hid_close_interrupt(void *opaque)
> -{
> - struct bt_hid_device_s *hid = opaque;
> -
> - hid->interrupt = NULL;
> - bt_hid_connected_update(hid);
> -}
> -
> -static int bt_hid_new_control_ch(struct bt_l2cap_device_s *dev,
> - struct bt_l2cap_conn_params_s *params)
> -{
> - struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
> -
> - if (hid->control)
> - return 1;
> -
> - hid->control = params;
> - hid->control->opaque = hid;
> - hid->control->close = bt_hid_close_control;
> - hid->control->sdu_in = bt_hid_control_sdu;
> -
> - bt_hid_connected_update(hid);
> -
> - return 0;
> -}
> -
> -static int bt_hid_new_interrupt_ch(struct bt_l2cap_device_s *dev,
> - struct bt_l2cap_conn_params_s *params)
> -{
> - struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
> -
> - if (hid->interrupt)
> - return 1;
> -
> - hid->interrupt = params;
> - hid->interrupt->opaque = hid;
> - hid->interrupt->close = bt_hid_close_interrupt;
> - hid->interrupt->sdu_in = bt_hid_interrupt_sdu;
> -
> - bt_hid_connected_update(hid);
> -
> - return 0;
> -}
> -
> -static void bt_hid_destroy(struct bt_device_s *dev)
> -{
> - struct bt_hid_device_s *hid = (struct bt_hid_device_s *) dev;
> -
> - if (hid->connected)
> - bt_hid_send_control(hid, BT_HC_VIRTUAL_CABLE_UNPLUG);
> - bt_l2cap_device_done(&hid->btdev);
> -
> - hid_free(&hid->hid);
> -
> - g_free(hid);
> -}
> -
> -enum peripheral_minor_class {
> - class_other = 0 << 4,
> - class_keyboard = 1 << 4,
> - class_pointing = 2 << 4,
> - class_combo = 3 << 4,
> -};
> -
> -static struct bt_device_s *bt_hid_init(struct bt_scatternet_s *net,
> - enum peripheral_minor_class minor)
> -{
> - struct bt_hid_device_s *s = g_malloc0(sizeof(*s));
> - uint32_t class =
> - /* Format type */
> - (0 << 0) |
> - /* Device class */
> - (minor << 2) |
> - (5 << 8) | /* "Peripheral" */
> - /* Service classes */
> - (1 << 13) | /* Limited discoverable mode */
> - (1 << 19); /* Capturing device (?) */
> -
> - bt_l2cap_device_init(&s->btdev, net);
> - bt_l2cap_sdp_init(&s->btdev);
> - bt_l2cap_psm_register(&s->btdev, BT_PSM_HID_CTRL,
> - BT_HID_MTU, bt_hid_new_control_ch);
> - bt_l2cap_psm_register(&s->btdev, BT_PSM_HID_INTR,
> - BT_HID_MTU, bt_hid_new_interrupt_ch);
> -
> - hid_init(&s->hid, HID_KEYBOARD, bt_hid_datain);
> - s->btdev.device.lmp_name = "BT Keyboard";
> -
> - s->btdev.device.handle_destroy = bt_hid_destroy;
> -
> - s->btdev.device.class[0] = (class >> 0) & 0xff;
> - s->btdev.device.class[1] = (class >> 8) & 0xff;
> - s->btdev.device.class[2] = (class >> 16) & 0xff;
> -
> - return &s->btdev.device;
> -}
> -
> -struct bt_device_s *bt_keyboard_init(struct bt_scatternet_s *net)
> -{
> - return bt_hid_init(net, class_keyboard);
> -}
> diff --git a/vl.c b/vl.c
> index 55bab005b6..2a12fc7f3e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -988,37 +988,6 @@ static void bt_vhci_add(int vlan_id)
> bt_vhci_init(bt_new_hci(vlan));
> }
>
> -static struct bt_device_s *bt_device_add(const char *opt)
> -{
> - struct bt_scatternet_s *vlan;
> - int vlan_id = 0;
> - char *endp = strstr(opt, ",vlan=");
> - int len = (endp ? endp - opt : strlen(opt)) + 1;
> - char devname[10];
> -
> - pstrcpy(devname, MIN(sizeof(devname), len), opt);
> -
> - if (endp) {
> - vlan_id = strtol(endp + 6, &endp, 0);
> - if (*endp) {
> - error_report("unrecognised bluetooth vlan Id");
> - return 0;
> - }
> - }
> -
> - vlan = qemu_find_bt_vlan(vlan_id);
> -
> - if (!vlan->slave)
> - warn_report("adding a slave device to an empty scatternet %i",
> - vlan_id);
> -
> - if (!strcmp(devname, "keyboard"))
> - return bt_keyboard_init(vlan);
> -
> - error_report("unsupported bluetooth device '%s'", devname);
> - return 0;
> -}
> -
> static int bt_parse(const char *opt)
> {
> const char *endp, *p;
> @@ -1051,8 +1020,7 @@ static int bt_parse(const char *opt)
> bt_vhci_add(vlan);
> return 0;
> }
> - } else if (strstart(opt, "device:", &endp))
> - return !bt_device_add(endp);
> + }
>
> error_report("bad bluetooth parameter '%s'", opt);
> return 1;
> diff --git a/hw/bt/Makefile.objs b/hw/bt/Makefile.objs
> index 867a7d2e8a..a33983b522 100644
> --- a/hw/bt/Makefile.objs
> +++ b/hw/bt/Makefile.objs
> @@ -1,3 +1,2 @@
> -common-obj-y += core.o l2cap.o sdp.o hci.o hid.o
> +common-obj-y += core.o l2cap.o sdp.o hci.o
> common-obj-y += hci-csr.o
> -
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 38c7a978c1..48885cdca8 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2804,15 +2804,6 @@ be used as following:
> qemu-system-i386 [...OPTIONS...] -bt hci,vlan=5 -bt vhci,vlan=5
> @end example
>
> -@item -bt device:@var{dev}[,vlan=@var{n}]
> -Emulate a bluetooth device @var{dev} and place it in network @var{n}
> -(default @code{0}). QEMU can only emulate one type of bluetooth devices
> -currently:
> -
> -@table @option
> -@item keyboard
> -Virtual wireless keyboard implementing the HIDP bluetooth profile.
> -@end table
> ETEXI
>
> STEXI
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
2018-11-09 19:02 ` Peter Maydell
@ 2018-11-12 9:16 ` Markus Armbruster
2018-11-12 10:19 ` Peter Maydell
0 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2018-11-12 9:16 UTC (permalink / raw)
To: Peter Maydell; +Cc: Gerd Hoffmann, Paolo Bonzini, QEMU Developers
Peter Maydell <peter.maydell@linaro.org> writes:
> On 9 November 2018 at 14:14, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> Broken (segfaultson first keypress) and appearently unused.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Please show the reproducer in the commit message. Stack backtrace
wouldn't hurt.
>> ---
>> include/hw/bt.h | 3 -
>> hw/bt/hid.c | 554 ----------------------------------------------------
>> vl.c | 34 +---
>> hw/bt/Makefile.objs | 3 +-
>> qemu-options.hx | 9 -
>> 5 files changed, 2 insertions(+), 601 deletions(-)
>> delete mode 100644 hw/bt/hid.c
>
> Are we definitely happy that all the use cases for
> this code segfault, not just the one you tested ?
Are there any others?
> Does it cost us much to mark this deprecated in 3.1
> and drop it in 4.0 ?
The cost is offering a known-to-be-broken option to users.
The other half of the tradeoff: what would it gain us?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
2018-11-09 14:14 [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation Gerd Hoffmann
` (4 preceding siblings ...)
2018-11-09 21:11 ` Paolo Bonzini
@ 2018-11-12 10:16 ` Thomas Huth
5 siblings, 0 replies; 10+ messages in thread
From: Thomas Huth @ 2018-11-12 10:16 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
Cc: Paolo Bonzini, Andrzej Zaborowski, Andrzej Zaborowski
On 2018-11-09 15:14, Gerd Hoffmann wrote:
> Broken (segfaultson first keypress) and appearently unused.
Broken since QEMU v2.1 (!) already, if I've got that right - please
mention that version number, so that it is clear that it is broken since
more than two releases (i.e. it has been "deprecated" by being broken
for so long).
There is also one more occurance of "-bt device:keyboard" in
qemu-doc.texi which you should now remove, too, I think.
Thomas
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
2018-11-12 9:16 ` Markus Armbruster
@ 2018-11-12 10:19 ` Peter Maydell
2018-11-12 14:47 ` Markus Armbruster
0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2018-11-12 10:19 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Gerd Hoffmann, Paolo Bonzini, QEMU Developers
On 12 November 2018 at 09:16, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 9 November 2018 at 14:14, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>> Broken (segfaultson first keypress) and appearently unused.
>>>
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Please show the reproducer in the commit message. Stack backtrace
> wouldn't hurt.
>
>>> ---
>>> include/hw/bt.h | 3 -
>>> hw/bt/hid.c | 554 ----------------------------------------------------
>>> vl.c | 34 +---
>>> hw/bt/Makefile.objs | 3 +-
>>> qemu-options.hx | 9 -
>>> 5 files changed, 2 insertions(+), 601 deletions(-)
>>> delete mode 100644 hw/bt/hid.c
>>
>> Are we definitely happy that all the use cases for
>> this code segfault, not just the one you tested ?
>
> Are there any others?
>
>> Does it cost us much to mark this deprecated in 3.1
>> and drop it in 4.0 ?
>
> The cost is offering a known-to-be-broken option to users.
>
> The other half of the tradeoff: what would it gain us?
My point is that we do not know it to be broken. We know that
it has one bug, in the command line and segfault that Gerd
found. I haven't seen anybody say they've done a check of
all the use cases of this code and confirmed that all of them
must pass through the codepath/situation that segfaults.
What it gains us is that we get to be sure that we are keeping
our promise with our users that we don't just randomly remove
features they were using without notice.
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation.
2018-11-12 10:19 ` Peter Maydell
@ 2018-11-12 14:47 ` Markus Armbruster
0 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2018-11-12 14:47 UTC (permalink / raw)
To: Peter Maydell; +Cc: Paolo Bonzini, Gerd Hoffmann, QEMU Developers
Peter Maydell <peter.maydell@linaro.org> writes:
> On 12 November 2018 at 09:16, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> On 9 November 2018 at 14:14, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>>> Broken (segfaultson first keypress) and appearently unused.
>>>>
>>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>
>> Please show the reproducer in the commit message. Stack backtrace
>> wouldn't hurt.
>>
>>>> ---
>>>> include/hw/bt.h | 3 -
>>>> hw/bt/hid.c | 554 ----------------------------------------------------
>>>> vl.c | 34 +---
>>>> hw/bt/Makefile.objs | 3 +-
>>>> qemu-options.hx | 9 -
>>>> 5 files changed, 2 insertions(+), 601 deletions(-)
>>>> delete mode 100644 hw/bt/hid.c
>>>
>>> Are we definitely happy that all the use cases for
>>> this code segfault, not just the one you tested ?
>>
>> Are there any others?
>>
>>> Does it cost us much to mark this deprecated in 3.1
>>> and drop it in 4.0 ?
>>
>> The cost is offering a known-to-be-broken option to users.
>>
>> The other half of the tradeoff: what would it gain us?
>
> My point is that we do not know it to be broken. We know that
> it has one bug, in the command line and segfault that Gerd
> found. I haven't seen anybody say they've done a check of
> all the use cases of this code and confirmed that all of them
> must pass through the codepath/situation that segfaults.
There's just one way to create a Bluetooth keyboard device. It has just
one configuration knob: the Bluetooh "vlan". This leads me to believe
there's no way to create this device that doesn't crash. Regardless,
asking Gerd to confirm is fair. Preferably in the commit message of v2.
> What it gains us is that we get to be sure that we are keeping
> our promise with our users that we don't just randomly remove
> features they were using without notice.
No gain unless there is a way to use it.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-11-12 14:48 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09 14:14 [Qemu-devel] [PATCH] hw/bt: drop bluetooth keyboard emulation Gerd Hoffmann
2018-11-09 14:52 ` Liam Merwick
2018-11-09 16:36 ` Eric Blake
2018-11-09 19:02 ` Peter Maydell
2018-11-12 9:16 ` Markus Armbruster
2018-11-12 10:19 ` Peter Maydell
2018-11-12 14:47 ` Markus Armbruster
2018-11-09 19:36 ` Philippe Mathieu-Daudé
2018-11-09 21:11 ` Paolo Bonzini
2018-11-12 10:16 ` Thomas Huth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.