Skip to content

platform-intel: fix handling of VMD drives behind PCH #276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gardzinx wants to merge 1 commit into
base: main
Choose a base branch
from
Open

platform-intel: fix handling of VMD drives behind PCH #276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 50 additions & 7 deletions platform-intel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ static void free_sys_dev(struct sys_dev **list)

if ((*list)->path)
free((*list)->path);
if ((*list)->path_pch)
free((*list)->path_pch);
free(*list);
*list = next;
}
Expand All @@ -116,14 +118,14 @@ static void free_sys_dev(struct sys_dev **list)
* IMSM must know VMD domains, therefore it needs to determine and follow buses.
*
*/
mdadm_status_t vmd_find_pci_bus(char *vmd_path, char *buf)
mdadm_status_t vmd_find_pci_bus(char *vmd_path, char *buf, char *domain)
{
char tmp[PATH_MAX];
struct dirent *ent;
DIR *vmd_dir;
char *rp_ret;

snprintf(tmp, PATH_MAX, "%s/domain/device", vmd_path);
snprintf(tmp, PATH_MAX, "%s/%s/device", vmd_path, domain);

rp_ret = realpath(tmp, buf);

Expand Down Expand Up @@ -175,6 +177,9 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
/* search sysfs for devices driven by 'driver' */
char path[PATH_MAX];
char link[PATH_MAX];
char *domain_cpu = "domain";
char *domain_pch = "domain1";
bool domain_pch_exists = false;
char *c;
DIR *driver_dir;
struct dirent *de;
Expand Down Expand Up @@ -241,7 +246,8 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
continue;
}
for (dev = vmd; dev; dev = dev->next) {
if ((strncmp(dev->path, rp, strlen(dev->path)) == 0))
if ((strncmp(dev->path, rp, strlen(dev->path)) == 0) ||
(strncmp(dev->path_pch, rp, strlen(dev->path)) == 0))
skip = 1;
}
free(rp);
Expand Down Expand Up @@ -276,11 +282,16 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
continue;

if (type == SYS_DEV_VMD) {
char vmd_path[PATH_MAX];
char vmd_path[PATH_MAX - 8];
char vmd_path_pch[PATH_MAX];

sprintf(vmd_path, "/sys/bus/%s/drivers/%s/%s", bus, driver, de->d_name);

if (vmd_find_pci_bus(vmd_path, path)) {
sprintf(vmd_path_pch, "%s/%s", vmd_path, domain_pch);
if (access(vmd_path_pch, F_OK) == 0)
domain_pch_exists = true;

if (vmd_find_pci_bus(vmd_path, path, domain_cpu)) {
pr_err("Cannot determine VMD bus for %s\n", vmd_path);
continue;
}
Expand Down Expand Up @@ -312,9 +323,32 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
list->type = type;
list->next = NULL;
list->path = p;
list->path_pch = NULL;

if ((list->pci_id = strrchr(list->path, '/')) != NULL)
list->pci_id++;

/* if its VMD device and path to PCH domain exists,
* assign the pch path to the vmd node
*/
if (domain_pch_exists && type == SYS_DEV_VMD) {
char vmd_path[PATH_MAX];

sprintf(path, "/sys/bus/%s/drivers/%s/%s",
bus, driver, de->d_name);
sprintf(vmd_path, "/sys/bus/%s/drivers/%s/%s",
bus, driver, de->d_name);
if (vmd_find_pci_bus(vmd_path, path, domain_pch)) {
pr_err("Cannot determine VMD bus for %s\n", vmd_path);
continue;
}
p = realpath(path, NULL);
if (!p) {
pr_err("Unable to get real path for '%s'\n", path);
continue;
} else
list->path_pch = p;
}
}
closedir(driver_dir);

Expand Down Expand Up @@ -1398,6 +1432,8 @@ char *vmd_domain_to_controller(struct sys_dev *hba, char *buf)
struct dirent *ent;
DIR *dir;
char path[PATH_MAX];
char *domain_cpu = "domain";
char *domain_pch = "domain1";

if (!hba)
return NULL;
Expand All @@ -1410,8 +1446,15 @@ char *vmd_domain_to_controller(struct sys_dev *hba, char *buf)
return NULL;

for (ent = readdir(dir); ent; ent = readdir(dir)) {
sprintf(path, "/sys/bus/pci/drivers/vmd/%s/domain/device",
ent->d_name);
/* pci_id for cpu vmd is pci10000:e0 and its domain is 'domain'
* while pch vmd is pci10000:e1 and its domain is 'domain1'
*/
if (hba->pci_id[strlen(hba->pci_id) - 1] == '0')
sprintf(path, "/sys/bus/pci/drivers/vmd/%s/%s/device",
ent->d_name, domain_cpu);
else
sprintf(path, "/sys/bus/pci/drivers/vmd/%s/%s/device",
ent->d_name, domain_pch);

if (!realpath(path, buf))
continue;
Expand Down
1 change: 1 addition & 0 deletions platform-intel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ enum sys_dev_type {
struct sys_dev {
enum sys_dev_type type;
char *path;
char *path_pch;
char *pci_id;

@mtkaczyk mtkaczyk Jun 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly speaking, this is approach is opposite to the design. In mdadm we have sys_dev which should describe each individual device with unique pci_id. To me it appears that domain1 pci_id is different than domain and you are workaround that. Am I correct?

How we would support "domain2" in the future? It it not extensible.

I don't look enough detailed to suggest you the correct way of implementing it but definetelly it is not something that can be accepted without good explanation.

please also avoid platform specyfic things like "pch" in a general code. It might be reused in different context in the future.

__u16 dev_id;
__u32 class;
Expand Down
3 changes: 2 additions & 1 deletion super-intel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2591,7 +2591,8 @@ static int print_nvme_info(struct sys_dev *hba)
!diskfd_to_devpath(fd, 1, cntrl_path))
goto skip;

if (!is_path_attached_to_hba(cntrl_path, hba->path))
if (!(is_path_attached_to_hba(cntrl_path, hba->path) ||
is_path_attached_to_hba(cntrl_path, hba->path_pch)))
goto skip;

if (!imsm_is_nvme_namespace_supported(fd, 0))
Expand Down