Paths

Table of Contentst

-
usr.sbin/bhyve/
-
bhyve/
2/10
pci_gvt-d.c

bhyve: read OpRegion address and size for GVT-d
ClosedPublic
Actions

Authored by corvink on May 10 2023, 12:45 PM.

Details

Reviewers

jhb
markj

Group Reviewers

bhyve

Commits

rGb260e413b4b7: bhyve: read OpRegion address and size for GVT-d
rG6952b9d25e18: bhyve: read OpRegion address and size for GVT-d

Summary

The OpRegion provides some configuration bits and ACPI methods used by
some Intel drivers. The guest needs access to it. In the first step,
we're reading it's address and size.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 51469
Build 48360: arc lint + arc unit

Event Timeline

corvink created this revision.May 10 2023, 12:45 PM

Herald added subscribers: bcran, rgrimes, imp. · View Herald TranscriptMay 10 2023, 12:45 PM

corvink requested review of this revision.May 10 2023, 12:45 PM

Harbormaster completed remote builds in B51414: Diff 121796.May 10 2023, 12:45 PM

corvink added a child revision: D40041: bhyve: copy OpRegion into guest memory.May 10 2023, 12:51 PM

corvink added a parent revision: D40039: bhyve: read out graphics stolen memory address and size.

markj added inline comments.May 10 2023, 5:49 PM

usr.sbin/bhyve/pci_gvt-d.c
167	It can't be O_RDONLY?
200	`memfd` is leaked here, no? We should be able to close it after mapping the header, I believe. In general we don't want to leave a `fd` for /dev/mem lying around.

don't leak memfd
open memfd readonly

Harbormaster completed remote builds in B51442: Diff 121847.May 11 2023, 10:38 AM

corvink added a parent revision: D40060: bhyve: emulate graphics stolen memory register.May 11 2023, 10:39 AM

corvink removed a parent revision: D40039: bhyve: read out graphics stolen memory address and size.

corvink added a parent revision: D40037: bhyve: import OpRegion definitions.May 11 2023, 12:32 PM

markj added inline comments.May 11 2023, 2:55 PM

usr.sbin/bhyve/pci_gvt-d.c
185
187
189	The header is guaranteed to be page-aligned?

corvink added inline comments.May 12 2023, 12:17 PM

usr.sbin/bhyve/pci_gvt-d.c
189	No, it doesn't have to be page aligned. On my systems it has an offset of +0x18 to page boundary. Don't know why.

don't return errno
make use of warn instead of warnx

Harbormaster completed remote builds in B51469: Diff 121907.May 12 2023, 12:24 PM

corvink marked 4 inline comments as done.May 12 2023, 12:24 PM

@jhb @markj Any additional comments?

jhb accepted this revision.Jun 7 2023, 6:35 PM

This revision is now accepted and ready to land.Jun 7 2023, 6:35 PM

Closed by commit rG6952b9d25e18: bhyve: read OpRegion address and size for GVT-d (authored by corvink). · Explain WhyJun 16 2023, 6:15 AM

This revision was automatically updated to reflect the committed changes.

corvink added a commit: rG6952b9d25e18: bhyve: read OpRegion address and size for GVT-d.

corvink added a commit: rGb260e413b4b7: bhyve: read OpRegion address and size for GVT-d.Aug 18 2023, 7:43 AM

Revision Contents
Changeset List

Path

Size

usr.sbin/

bhyve/

pci_gvt-d.c

67 lines

Diff 121907

View Options

usr.sbin/bhyve/pci_gvt-d.c

Show All 36 Lines

#include <dev/smbus/smbconf.h>		#include <dev/smbus/smbconf.h>
#include <dev/smbus/smbus.h>		#include <dev/smbus/smbus.h>

#include "smbus_if.h"		#include "smbus_if.h"


/*		/*
		* General device identification notes.
		*
		* The JEDEC TSE2004av specification defines the device ID that all compliant
		* devices should use, but very few do in practice. Maybe that's because
		* TSE2002av was rather vague about that.
		* Rare examples are IDT TSE2004GB2B0 and Atmel AT30TSE004A, not sure if
		* they are TSE2004av compliant by design or by accident.
		* Also, the specification mandates that PCI SIG manufacturer IDs are to be
		* used, but in practice the JEDEC manufacturer IDs are often used.
		*/
		rpokalaUnsubmitted Not Done Inline Actions Add a note saying that the spec requires the upper byte of the device_id be 0x22, but that most vendors don't honor that, so that's why we're not checking for it. That makes the 0x22 in the generic case a bit less magic. rpokala: Add a note saying that the spec requires the upper byte of the device_id be 0x22, but that most…
		avgAuthorUnsubmitted Not Done Inline Actions Isn't this comment the comment that you are requesting? avg: Isn't this comment the comment that you are requesting?
		rpokalaUnsubmitted Not Done Inline Actions Err, yes - partly. It mentions that few use the proper one. How about: ... devices should use (high-order byte 0x22), but very few do in practice ... rpokala: Err, yes - partly. It mentions that few use the proper one. How about: > ... devices should…
		avgAuthorUnsubmitted Not Done Inline Actions I think that this comment is fine as is. There is another comment in the code where the magic device ID is actually used. avg: I think that this comment is fine as is. There is another comment in the code where the magic…

		const struct ts_dev {
		uint16_t vendor_mask;
		impUnsubmitted Done Inline Actions Since the vendor mask is always 0xffff, why have it? It only seems useful for the catch-all at the end... imp: Since the vendor mask is always 0xffff, why have it? It only seems useful for the catch-all at…
		uint16_t vendor_id;
		uint16_t device_mask;
		impUnsubmitted Done Inline Actions device_mask is likewise always 0xff00. Looks like the lower 8 bits are supposed to be a revision number. I see why you want to mask it, but realistically, will there ever be a time we'd have two different entries here? imp: device_mask is likewise always 0xff00. Looks like the lower 8 bits are supposed to be a…
		rpokalaUnsubmitted Not Done Inline Actions JEDEC Standard No. 21-C 2.24. TSE2004av Device ID / Revision Register The upper byte of the Device ID / Revision Register must be 0x22 for the TSE2004av. The lower byte holds the revision value which is vendor-specific. This register is not available on EE1004-v devices. Based on that, we should be looking at all four nybbles, which should be 0x22XX. But as pointed out in the comment, not everyone uses the correct device ID. Since it's the low-order bits which are vendor-specific, shouldn't we only look at those for identification? (But maybe the vendor datasheets indicate that the low-order bits might be different even for the same part? I haven't looked at them yet; I'll do that later today. If that's the case, then I'd say we shouldn't look at the device-id at all.) rpokala: > JEDEC Standard No. 21-C > 2.24. TSE2004av Device ID / Revision Register > The upper byte of…
		avgAuthorUnsubmitted Not Done Inline Actions The low bits are "volatile". Most vendor specifications say something to the effect that the revision is X in this version of the product and will be incremented in subsequent versions. Anyway, this is mostly moot as of all the devices out there I found only two that have 0x22 in the upper half. avg: The low bits are "volatile". Most vendor specifications say something to the effect that the…
		impUnsubmitted Not Done Inline Actions Right, hence my comment: we should just have a blanket policy of ignoring them and have no need for it in this table. imp: Right, hence my comment: we should just have a blanket policy of ignoring them and have no need…
		avgAuthorUnsubmitted Not Done Inline Actions I removed the generic device ID from the table... I added it as a special case to `ts_match_device` though. avg: I removed the generic device ID from the table... I added it as a special case to…
		uint16_t device_id;
		const char *description;
		} known_devices[] = {
		/*
		* Analog Devices ADT7408.
		* http://www.analog.com/media/en/technical-documentation/data-sheets/ADT7408.pdf
		*/
		{
		0xffff, 0x11d4, 0xff00, 0x0800,
		"Analog Devices DIMM temperature sensor"
		},

		/*
		* Atmel AT30TSE002B, AT30TSE004A.
		* http://www.atmel.com/images/doc8711.pdf
		* http://www.atmel.com/images/atmel-8868-dts-at30tse004a-datasheet.pdf
		* Note how one chip uses the JEDEC Manufacturer ID while the other
		* uses the PCI SIG one.
		*/
		{
		0xffff, 0x001f, 0xff00, 0x8200,
		"Atmel DIMM temperature sensor"
		},
		{
		0xffff, 0x1114, 0xff00, 0x2200,
		"Atmel DIMM temperature sensor"
		},

		/*
		* Integrated Device Technology (IDT) TS3000B3A, TSE2002B3C,
		* TSE2004GB2B0 chips and their variants.
		* http://www.idt.com/sites/default/files/documents/IDT_TSE2002B3C_DST_20100512_120303152056.pdf
		* http://www.idt.com/sites/default/files/documents/IDT_TS3000B3A_DST_20101129_120303152013.pdf
		* https://www.idt.com/document/dst/tse2004gb2b0-datasheet
		*/
		{
		0xffff, 0x00b3, 0xff00, 0x2900,
		"IDT DIMM temperature sensor"
		},
		{
		0xffff, 0x00b3, 0xff00, 0x2200,
		"IDT DIMM temperature sensor"
		},

		/*
		* Maxim Integrated MAX6604.
		* Different document revisions specify different Device IDs.
		* Document 19-3837; Rev 0; 10/05 has 0x3e00 while
		* 19-3837; Rev 3; 10/11 has 0x5400.
		* http://datasheets.maximintegrated.com/en/ds/MAX6604.pdf
		*/
		{
		0xffff, 0x004d, 0xff00, 0x3e00,
		"Maxim Integrated DIMM temperature sensor"
		},
		{
		0xffff, 0x004d, 0xff00, 0x5400,
		"Maxim Integrated DIMM temperature sensor"
		},

		/*
		* Microchip Technology MCP9805, MCP9843, MCP98242, MCP98243
		* and their variants.
		* http://ww1.microchip.com/downloads/en/DeviceDoc/21977b.pdf
		* Microchip Technology EMC1501.
		* http://ww1.microchip.com/downloads/en/DeviceDoc/00001605A.pdf
		*/
		{
		0xffff, 0x0054, 0xff00, 0x0000,
		"Microchip DIMM temperature sensor"
		},
		{
		0xffff, 0x0054, 0xff00, 0x2000,
		"Microchip DIMM temperature sensor"
		},
		{
		0xffff, 0x0054, 0xff00, 0x2100,
		"Microchip DIMM temperature sensor"
		},
		{
		0xffff, 0x1055, 0xff00, 0x0800,
		"Microchip DIMM temperature sensor"
		},

		/*
		* NXP Semiconductors SE97 and SE98.
		* http://www.nxp.com/docs/en/data-sheet/SE97B.pdf
		*/
		{
		0xffff, 0x1131, 0xff00, 0xa100,
		"NXP DIMM temperature sensor"
		},
		{
		0xffff, 0x1131, 0xff00, 0xa200,
		"NXP DIMM temperature sensor"
		},

		/*
		* ON Semiconductor CAT34TS02 revisions B and C, CAT6095 and compatible.
		* https://www.onsemi.com/pub/Collateral/CAT34TS02-D.PDF
		* http://www.onsemi.com/pub/Collateral/CAT6095-D.PDF
		*/
		{
		0xffff, 0x1b09, 0xff00, 0x0800,
		"ON Semiconductor DIMM temperature sensor"
		},
		{
		0xffff, 0x1b09, 0xff00, 0x0a00,
		"ON Semiconductor DIMM temperature sensor"
		},

		/*
		* ST[Microelectronics] STTS424E02, STTS2002 and others.
		* http://www.st.com/resource/en/datasheet/cd00157558.pdf
		* http://www.st.com/resource/en/datasheet/stts2002.pdf
		*/
		{
		0xffff, 0x104a, 0xff00, 0x0000,
		"ST DIMM temperature sensor"
		},
		{
		0xffff, 0x104a, 0xff00, 0x0300,
		"ST DIMM temperature sensor"
		},

		/* Generic TSE2004av compliant device. */
		{
		0x0000, 0x0000, 0xff00, 0x2200,
		"TSE2004av compliant DIMM temperature sensor"
		},
		};

		static const struct ts_dev *
		ts_match_device(uint16_t vid, uint16_t did)
		{
		const struct ts_dev *d;
		int i;

		for (i = 0; i < nitems(known_devices); i++) {
		d = &known_devices[i];
		if ((vid & d->vendor_mask) == d->vendor_id &&
		(did & d->device_mask) == d->device_id) {
		return (d);
		}
		}
		return (NULL);
		}

		/*
* SMBus specification defines little-endian byte order,		* SMBus specification defines little-endian byte order,
* but it seems that the JEDEC devices expect it to		* but it seems that the JEDEC devices expect it to
* be big-endian.		* be big-endian.
*/		*/
static int		static int
ts_readw_be(device_t dev, uint8_t reg, uint16_t *val)		ts_readw_be(device_t dev, uint8_t reg, uint16_t *val)
{		{
device_t bus = device_get_parent(dev);		device_t bus = device_get_parent(dev);
Show All 31 Lines	ts_temp_sysctl(SYSCTL_HANDLER_ARGS)
temp = temp * 625 + 2731500;		temp = temp * 625 + 2731500;
err = sysctl_handle_int(oidp, &temp, 0, req);		err = sysctl_handle_int(oidp, &temp, 0, req);
return (err);		return (err);
}		}

static int		static int
ts_probe(device_t dev)		ts_probe(device_t dev)
{		{
device_set_desc(dev, "DIMM memory sensor");
return (BUS_PROBE_DEFAULT);		return (BUS_PROBE_DEFAULT);
}		}

static int		static int
ts_attach(device_t dev)		ts_attach(device_t dev)
{		{
struct sysctl_ctx_list *ctx;		struct sysctl_ctx_list *ctx;
struct sysctl_oid_list *tree;		struct sysctl_oid_list *tree;
		const struct ts_dev *match;
int err;		int err;
uint16_t vendorid;		uint16_t vendorid;
uint16_t devid;		uint16_t devid;
uint8_t addr;		uint8_t addr;

addr = smbus_get_addr(dev);		addr = smbus_get_addr(dev);
if ((addr & 0xf0) != 0x30) {		if ((addr & 0xf0) != 0x30) {
/* Up to 8 slave devices starting at 0x30. */		/* Up to 8 slave devices starting at 0x30. */
return (ENXIO);		return (ENXIO);
}		}

err = ts_readw_be(dev, 6, &vendorid);		err = ts_readw_be(dev, 6, &vendorid);
if (err != 0) {		if (err != 0) {
device_printf(dev, "failed to read Manufacturer ID\n");		device_printf(dev, "failed to read Manufacturer ID\n");
return (ENXIO);		return (ENXIO);
}		}
err = ts_readw_be(dev, 7, &devid);		err = ts_readw_be(dev, 7, &devid);
if (err != 0) {		if (err != 0) {
device_printf(dev, "failed to read Device ID\n");		device_printf(dev, "failed to read Device ID\n");
return (ENXIO);		return (ENXIO);
}		}
if ((devid & 0xff00) == 0x2200) {
/*		match = ts_match_device(vendorid, devid);
* Defined by JEDEC Standard No. 21-C, Release 26,		if (match == NULL) {
* Page 4.1.6 – 24
*/
} else if (vendorid == 0x104a) {
/*
* STMicroelectronics datasheets say that
* device ID and revision can vary.
* E.g. STT424E02, Doc ID 13448 Rev 8,
* section 4.6, page 26.
*/
} else if (vendorid == 0xb3 && (devid & 0xff00) == 0x2900) {
/*
* IDT TS3000B3A and TSE2002B3C chips and their variants.
* Revision IDs (the lower byte) can vary.
* http://www.idt.com/sites/default/files/documents/IDT_TSE2002B3C_DST_20100512_120303152056.pdf
* http://www.idt.com/sites/default/files/documents/IDT_TS3000B3A_DST_20101129_120303152013.pdf
*/
} else {
if (bootverbose) {		if (bootverbose) {
device_printf(dev, "Unknown Manufacturer and Device IDs"		device_printf(dev, "Unknown Manufacturer and Device IDs"
", 0x%x and 0x%x\n", vendorid, devid);		", 0x%x and 0x%x\n", vendorid, devid);
}		}
return (ENXIO);		return (ENXIO);
}		}

		device_set_desc(dev, match->description);

ctx = device_get_sysctl_ctx(dev);		ctx = device_get_sysctl_ctx(dev);
tree = SYSCTL_CHILDREN(device_get_sysctl_tree(dev));		tree = SYSCTL_CHILDREN(device_get_sysctl_tree(dev));

SYSCTL_ADD_PROC(ctx, tree, OID_AUTO, "temp",		SYSCTL_ADD_PROC(ctx, tree, OID_AUTO, "temp",
CTLTYPE_INT \| CTLFLAG_RD \| CTLFLAG_MPSAFE, dev, 0,		CTLTYPE_INT \| CTLFLAG_RD \| CTLFLAG_MPSAFE, dev, 0,
ts_temp_sysctl, "IK4", "Current temperature");		ts_temp_sysctl, "IK4", "Current temperature");

return (0);		return (0);
}		}

static int		static int
Show All 27 Lines

bhyve: read OpRegion address and size for GVT-dClosedPublicActions