Differential D37408 Diff 113265 usr.sbin/bhyve/pci_xhci.c

Changeset View

Standalone View

usr.sbin/bhyve/pci_xhci.c

Show First 20 Lines • Show All 562 Lines • ▼ Show 20 Lines	case 8:
break;		break;
case 12:		case 12:
/*		/*
* Port hardware LPM control register.		* Port hardware LPM control register.
* For USB3, this register is reserved.		* For USB3, this register is reserved.
*/		*/
p->porthlpmc = value;		p->porthlpmc = value;
break;		break;
		default:
		DPRINTF(("pci_xhci: unaligned portreg write offset %#lx",
		offset));
		break;
}		}
}		}

static struct xhci_dev_ctx *		static struct xhci_dev_ctx *
pci_xhci_get_dev_ctx(struct pci_xhci_softc *sc, uint32_t slot)		pci_xhci_get_dev_ctx(struct pci_xhci_softc *sc, uint32_t slot)
{		{
uint64_t devctx_addr;		uint64_t devctx_addr;
struct xhci_dev_ctx *devctx;		struct xhci_dev_ctx *devctx;
▲ Show 20 Lines • Show All 1,545 Lines • ▼ Show 20 Lines	pci_xhci_rtsregs_write(struct pci_xhci_softc *sc, uint64_t offset,
}		}
}		}

static uint64_t		static uint64_t
pci_xhci_portregs_read(struct pci_xhci_softc *sc, uint64_t offset)		pci_xhci_portregs_read(struct pci_xhci_softc *sc, uint64_t offset)
{		{
struct pci_xhci_portregs *portregs;		struct pci_xhci_portregs *portregs;
int port;		int port;
uint32_t *p;		uint32_t reg;

if (sc->portregs == NULL)		if (sc->portregs == NULL)
return (0);		return (0);

port = (offset - 0x3F0) / 0x10;		port = (offset - XHCI_PORTREGS_PORT0) / XHCI_PORTREGS_SETSZ;
		offset = (offset - XHCI_PORTREGS_PORT0) % XHCI_PORTREGS_SETSZ;

if (port > XHCI_MAX_DEVS) {		if (port > XHCI_MAX_DEVS) {
DPRINTF(("pci_xhci: portregs_read port %d >= XHCI_MAX_DEVS",		DPRINTF(("pci_xhci: portregs_read port %d >= XHCI_MAX_DEVS",
port));		port));

/* return default value for unused port */		/* return default value for unused port */
return (XHCI_PS_SPEED_SET(3));		return (XHCI_PS_SPEED_SET(3));
}		}

offset = (offset - 0x3F0) % 0x10;

portregs = XHCI_PORTREG_PTR(sc, port);		portregs = XHCI_PORTREG_PTR(sc, port);
p = &portregs->portsc;		switch (offset) {
p += offset / sizeof(uint32_t);		case 0:
		reg = portregs->portsc;
		break;
		case 4:
		reg = portregs->portpmsc;
		break;
		case 8:
		reg = portregs->portli;
		break;
		case 12:
		reg = portregs->porthlpmc;
		break;
		default:
		corvinkUnsubmitted Not Done Inline Actions Should we add a default case? Something like: default: assert(false); break; corvink: Should we add a default case? Something like: ``` default: assert(false); break; ```
		jhbUnsubmitted Not Done Inline Actions We have an `__assert_unreachable()` macro in <sys/cdefs.h> in the kernel for these that uses the compiler's `__builtin_unreachable()`. We should use something similar for these sorts of cases rather than `assert(false)`. jhb: We have an `__assert_unreachable()` macro in <sys/cdefs.h> in the kernel for these that uses…
		markjAuthorUnsubmitted Done Inline Actions Is there anything preventing an unaligned write to a virtualized BAR? If not, then the default case is indeed reachable. We can log a warning and return 0 in that case. pci_xhci_portregs_write() currently silently ignores an unaligned write markj: Is there anything preventing an unaligned write to a virtualized BAR? If not, then the default…
		jhbUnsubmitted Not Done Inline Actions Hmm, well, these are reads, not writes, so we can't really ignore them (have to return something). The old code didn't ignore them, instead it "aligned" the read. That is since `offset / sizeof(uint32_t)` truncated in the old code, reading from offset 13 actually read the register at offset 12. We should either preserve that (e.g. `switch (offset & ~3)` or `switch (offset / sizeof(uint32_t)` and fixing the indices), or we should add a default case for misaligned reads. For invalid reads the normal behavior would be to return 0xffffffff rather than 0. jhb: Hmm, well, these are reads, not writes, so we can't really ignore them (have to return…
		corvinkUnsubmitted Not Done Inline Actions I'd add an assertion. This allows us in to easily detect guests which do unaligned accesses. If there are some guests doing unaligned accesses, we could discuss the correct return value. Maybe some strange guests read these register byte by byte, so we shouldn't return 0xff or 0 by default. `__builtin_unreachable()` is not applicable for this use case as it could be reachable. corvink: I'd add an assertion. This allows us in to easily detect guests which do unaligned accesses. If…
		markjAuthorUnsubmitted Done Inline Actions I suspect it doesn't make sense to bother aligning reads when writes are not similarly handled. I just added an explicit default case and a debug print so that the problem stands out when debugging the device model. markj: I suspect it doesn't make sense to bother aligning reads when writes are not similarly handled.
		DPRINTF(("pci_xhci: unaligned portregs read offset %#lx",
		offset));
		reg = 0xffffffff;
		break;
		}

DPRINTF(("pci_xhci: portregs read offset 0x%lx port %u -> 0x%x",		DPRINTF(("pci_xhci: portregs read offset 0x%lx port %u -> 0x%x",
offset, port, *p));		offset, port, reg));

return (*p);		return (reg);
}		}

static void		static void
pci_xhci_hostop_write(struct pci_xhci_softc *sc, uint64_t offset,		pci_xhci_hostop_write(struct pci_xhci_softc *sc, uint64_t offset,
uint64_t value)		uint64_t value)
{		{
offset -= XHCI_CAPLEN;		offset -= XHCI_CAPLEN;

▲ Show 20 Lines • Show All 1,030 Lines • Show Last 20 Lines