Skip to content

Commit

Permalink
vmm: Fix AMD-vi using wrong rid range
Browse files Browse the repository at this point in the history 
The ACPI parsing code around rid range was wrong on assuming there is
only one pair of start/end device id range. Besides, ivhd_dev_parse()
never work as supposed. The start/end rid info was always zero.

Restructure the code to build dynamic-sized tables for each IOMMU softc
holding device entries. The device entries are enumerated to find a
suitable IOMMU unit. Operations on devices not governed (e.g. the IOMMU
unit itself) are no-op from now on. There are also a minor fix on wrong
%b formatting string usage.

Tested on my EPYC 7282.

Sponsored by:	The FreeBSD Foundation
Reviewed by:	grehan
Differential Revision:	https://reviews.freebsd.org/D30827

(cherry picked from commit b5c74df)
  • Loading branch information
@khng300
khng300 committed Sep 3, 2021
1 parent da4e35d commit db877a0
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 63 deletions.
52 changes: 23 additions & 29 deletions sys/amd64/vmm/amd/amdvi_hw.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -811,11 +811,11 @@ amdvi_print_dev_cap(struct amdvi_softc *softc)

cfg = softc->dev_cfg;
for (i = 0; i < softc->dev_cfg_cnt; i++) {
device_printf(softc->dev, "device [0x%x - 0x%x]"
device_printf(softc->dev, "device [0x%x - 0x%x] "
"config:%b%s\n", cfg->start_id, cfg->end_id,
cfg->data,
"\020\001INIT\002ExtInt\003NMI"
"\007LINT0\008LINT1",
"\007LINT0\010LINT1",
cfg->enable_ats ? "ATS enabled" : "");
cfg++;
}
Expand Down Expand Up @@ -876,10 +876,6 @@ amdvi_add_sysctl(struct amdvi_softc *softc)
&softc->total_cmd, "Command submitted count");
SYSCTL_ADD_U16(ctx, child, OID_AUTO, "pci_rid", CTLFLAG_RD,
&softc->pci_rid, 0, "IOMMU RID");
SYSCTL_ADD_U16(ctx, child, OID_AUTO, "start_dev_rid", CTLFLAG_RD,
&softc->start_dev_rid, 0, "Start of device under this IOMMU");
SYSCTL_ADD_U16(ctx, child, OID_AUTO, "end_dev_rid", CTLFLAG_RD,
&softc->end_dev_rid, 0, "End of device under this IOMMU");
SYSCTL_ADD_PROC(ctx, child, OID_AUTO, "command_head",
CTLTYPE_UINT | CTLFLAG_RD, softc, 0,
amdvi_handle_sysctl, "IU", "Command head");
Expand Down Expand Up @@ -1207,22 +1203,17 @@ static struct amdvi_softc *
amdvi_find_iommu(uint16_t devid)
{
struct amdvi_softc *softc;
int i;
int i, j;

for (i = 0; i < ivhd_count; i++) {
softc = device_get_softc(ivhd_devs[i]);
if ((devid >= softc->start_dev_rid) &&
(devid <= softc->end_dev_rid))
return (softc);
for (j = 0; j < softc->dev_cfg_cnt; j++)
if ((devid >= softc->dev_cfg[j].start_id) &&
(devid <= softc->dev_cfg[j].end_id))
return (softc);
}

/*
* XXX: BIOS bug, device not in IVRS table, assume its from first IOMMU.
*/
printf("BIOS bug device(%d.%d.%d) doesn't have IVHD entry.\n",
RID2PCI_STR(devid));

return (device_get_softc(ivhd_devs[0]));
return (NULL);
}

/*
Expand All @@ -1231,14 +1222,12 @@ amdvi_find_iommu(uint16_t devid)
* be set concurrently, e.g. read and write bits.
*/
static void
amdvi_set_dte(struct amdvi_domain *domain, uint16_t devid, bool enable)
amdvi_set_dte(struct amdvi_domain *domain, struct amdvi_softc *softc,
uint16_t devid, bool enable)
{
struct amdvi_softc *softc;
struct amdvi_dte* temp;

KASSERT(domain, ("domain is NULL for pci_rid:0x%x\n", devid));

softc = amdvi_find_iommu(devid);
KASSERT(softc, ("softc is NULL for pci_rid:0x%x\n", devid));

temp = &amdvi_dte[devid];
Expand Down Expand Up @@ -1272,11 +1261,8 @@ amdvi_set_dte(struct amdvi_domain *domain, uint16_t devid, bool enable)
}

static void
amdvi_inv_device(uint16_t devid)
amdvi_inv_device(struct amdvi_softc *softc, uint16_t devid)
{
struct amdvi_softc *softc;

softc = amdvi_find_iommu(devid);
KASSERT(softc, ("softc is NULL"));

amdvi_cmd_inv_dte(softc, devid);
Expand All @@ -1291,29 +1277,37 @@ static void
amdvi_add_device(void *arg, uint16_t devid)
{
struct amdvi_domain *domain;
struct amdvi_softc *softc;

domain = (struct amdvi_domain *)arg;
KASSERT(domain != NULL, ("domain is NULL"));
#ifdef AMDVI_DEBUG_CMD
printf("Assigning device(%d.%d.%d) to domain:%d\n",
RID2PCI_STR(devid), domain->id);
#endif
amdvi_set_dte(domain, devid, true);
amdvi_inv_device(devid);
softc = amdvi_find_iommu(devid);
if (softc == NULL)
return;
amdvi_set_dte(domain, softc, devid, true);
amdvi_inv_device(softc, devid);
}

static void
amdvi_remove_device(void *arg, uint16_t devid)
{
struct amdvi_domain *domain;
struct amdvi_softc *softc;

domain = (struct amdvi_domain *)arg;
#ifdef AMDVI_DEBUG_CMD
printf("Remove device(0x%x) from domain:%d\n",
devid, domain->id);
#endif
amdvi_set_dte(domain, devid, false);
amdvi_inv_device(devid);
softc = amdvi_find_iommu(devid);
if (softc == NULL)
return;
amdvi_set_dte(domain, softc, devid, false);
amdvi_inv_device(softc, devid);
}

static void
Expand Down
13 changes: 7 additions & 6 deletions sys/amd64/vmm/amd/amdvi_priv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2016 Anish Gupta (anish@freebsd.org)
* All rights reserved.
* Copyright (c) 2021 The FreeBSD Foundation
*
* Portions of this software were developed by Ka Ho Ng
* under sponsorship from the FreeBSD Foundation.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -392,13 +395,11 @@ struct amdvi_softc {
uint8_t pci_cap; /* PCI capability. */
uint16_t pci_seg; /* IOMMU PCI domain/segment. */
uint16_t pci_rid; /* PCI BDF of IOMMU */
/* Device range under this IOMMU. */
uint16_t start_dev_rid; /* First device under this IOMMU. */
uint16_t end_dev_rid; /* Last device under this IOMMU. */

/* BIOS provided device configuration for end points. */
struct ivhd_dev_cfg dev_cfg[10];
/* ACPI device configuration for end points. */
struct ivhd_dev_cfg *dev_cfg;
int dev_cfg_cnt;
int dev_cfg_cap;

/* Software statistics. */
uint64_t event_intr_cnt; /* Total event INTR count. */
Expand Down
68 changes: 40 additions & 28 deletions sys/amd64/vmm/amd/ivrs_drv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,17 @@ ivhd_dev_add_entry(struct amdvi_softc *softc, uint32_t start_id,
{
struct ivhd_dev_cfg *dev_cfg;

/* If device doesn't have special data, don't add it. */
if (!cfg)
return;
KASSERT(softc->dev_cfg_cap <= softc->dev_cfg_cnt,
("Impossible case: number of dev_cfg exceeding capacity"));
if (softc->dev_cfg_cap == softc->dev_cfg_cnt) {
if (softc->dev_cfg_cap == 0)
softc->dev_cfg_cap = 1;
else
softc->dev_cfg_cap <<= 2;
softc->dev_cfg = realloc(softc->dev_cfg,
sizeof(*softc->dev_cfg) * softc->dev_cfg_cap, M_DEVBUF,
M_WAITOK);
}

dev_cfg = &softc->dev_cfg[softc->dev_cfg_cnt++];
dev_cfg->start_id = start_id;
Expand All @@ -204,14 +212,11 @@ ivhd_dev_parse(ACPI_IVRS_HARDWARE1 *ivhd, struct amdvi_softc *softc)
{
ACPI_IVRS_DE_HEADER *de;
uint8_t *p, *end;
int range_start_id = 0, range_end_id = 0;
int range_start_id = -1, range_end_id = -1, i;
uint32_t *extended;
uint8_t all_data = 0, range_data = 0;
bool range_enable_ats = false, enable_ats;

softc->start_dev_rid = ~0;
softc->end_dev_rid = 0;

switch (ivhd->Header.Type) {
case IVRS_TYPE_HARDWARE_LEGACY:
p = (uint8_t *)ivhd + sizeof(ACPI_IVRS_HARDWARE1);
Expand All @@ -232,11 +237,11 @@ ivhd_dev_parse(ACPI_IVRS_HARDWARE1 *ivhd, struct amdvi_softc *softc)

while (p < end) {
de = (ACPI_IVRS_DE_HEADER *)p;
softc->start_dev_rid = MIN(softc->start_dev_rid, de->Id);
softc->end_dev_rid = MAX(softc->end_dev_rid, de->Id);
switch (de->Type) {
case ACPI_IVRS_TYPE_ALL:
all_data = de->DataSetting;
for (i = 0; i < softc->dev_cfg_cnt; i++)
softc->dev_cfg[i].data |= all_data;
break;

case ACPI_IVRS_TYPE_SELECT:
Expand All @@ -256,6 +261,11 @@ ivhd_dev_parse(ACPI_IVRS_HARDWARE1 *ivhd, struct amdvi_softc *softc)
case ACPI_IVRS_TYPE_START:
case ACPI_IVRS_TYPE_ALIAS_START:
case ACPI_IVRS_TYPE_EXT_START:
if (range_start_id != -1) {
device_printf(softc->dev,
"Unexpected start-of-range device entry\n");
return (EINVAL);
}
range_start_id = de->Id;
range_data = de->DataSetting;
if (de->Type == ACPI_IVRS_TYPE_EXT_START) {
Expand All @@ -267,10 +277,20 @@ ivhd_dev_parse(ACPI_IVRS_HARDWARE1 *ivhd, struct amdvi_softc *softc)
break;

case ACPI_IVRS_TYPE_END:
if (range_start_id == -1) {
device_printf(softc->dev,
"Unexpected end-of-range device entry\n");
return (EINVAL);
}
range_end_id = de->Id;
if (range_end_id < range_start_id) {
device_printf(softc->dev,
"Device entry range going backward\n");
return (EINVAL);
}
ivhd_dev_add_entry(softc, range_start_id, range_end_id,
range_data | all_data, range_enable_ats);
range_start_id = range_end_id = 0;
range_data | all_data, range_enable_ats);
range_start_id = range_end_id = -1;
range_data = 0;
all_data = 0;
break;
Expand All @@ -288,12 +308,6 @@ ivhd_dev_parse(ACPI_IVRS_HARDWARE1 *ivhd, struct amdvi_softc *softc)
"Unknown dev entry:0x%x\n", de->Type);
}

if (softc->dev_cfg_cnt >
(sizeof(softc->dev_cfg) / sizeof(softc->dev_cfg[0]))) {
device_printf(softc->dev,
"WARN Too many device entries.\n");
return (EINVAL);
}
if (de->Type < 0x40)
p += sizeof(ACPI_IVRS_DEVICE4);
else if (de->Type < 0x80)
Expand All @@ -305,10 +319,6 @@ ivhd_dev_parse(ACPI_IVRS_HARDWARE1 *ivhd, struct amdvi_softc *softc)
}
}

KASSERT((softc->end_dev_rid >= softc->start_dev_rid),
("Device end[0x%x] < start[0x%x.\n",
softc->end_dev_rid, softc->start_dev_rid));

return (0);
}

Expand Down Expand Up @@ -620,9 +630,6 @@ ivhd_print_cap(struct amdvi_softc *softc, ACPI_IVRS_HARDWARE1 * ivhd)
max_ptp_level, amdvi_ptp_level);
}

device_printf(softc->dev, "device range: 0x%x - 0x%x\n",
softc->start_dev_rid, softc->end_dev_rid);

return (0);
}

Expand Down Expand Up @@ -680,21 +687,25 @@ ivhd_attach(device_t dev)
if (status != 0) {
device_printf(dev,
"endpoint device parsing error=%d\n", status);
goto fail;
}

status = ivhd_print_cap(softc, ivhd);
if (status != 0) {
return (status);
}
if (status != 0)
goto fail;

status = amdvi_setup_hw(softc);
if (status != 0) {
device_printf(dev, "couldn't be initialised, error=%d\n",
status);
return (status);
goto fail;
}

return (0);

fail:
free(softc->dev_cfg, M_DEVBUF);
return (status);
}

static int
Expand All @@ -705,6 +716,7 @@ ivhd_detach(device_t dev)
softc = device_get_softc(dev);

amdvi_teardown_hw(softc);
free(softc->dev_cfg, M_DEVBUF);

/*
* XXX: delete the device.
Expand Down

0 comments on commit db877a0

Please sign in to comment.