platform-intel: fix handling of VMD drives behind PCH by gardzinx · Pull Request #276 · md-raid-utilities/mdadm

gardzinx · 2026-06-18T07:23:20Z

In upcoming change, VMD will be working with PCH connected drives by assigning to them a separate domain ("domain1" as of writing this).
Currently mdadm will not see those drives as behind VMD and will list them in a separate driver incorrectly.

VMD drives behind PCH need to be handled the same way as the drives behind CPU - detect if PCH exists and detect drives on its path for the controller just like for CPU.

This new PCH domain might not be implemented yet but this patch should be backwards compatible.

Example output before this patch, ALL drives are set to be behind VMD:

Example output on platform:
# mdadm --detail-p
mdadm: imsm capabilities not found for controller: /sys/devices/pci0000:80/0000:80:17.0 (type SATA)
       Platform : Intel(R) Rapid Storage Technology enterprise
    RAID Levels : raid0 raid1 raid5 raid10
    Chunk Sizes : 4k 8k 16k 32k 64k 128k
    2TB volumes : supported
      2TB disks : supported
      Max Disks : 12
    Max Volumes : 2 per array, 12 per platform
3rd party NVMe : supported
    NVMe Device : /dev/nvme4n1 (CVFT4215005N400BGN)
                    Encryption(Ability|Status): None|Unencrypted
    NVMe Device : /dev/nvme1n1 (PHLF7365001H1P0GGN)
                    Encryption(Ability|Status): None|Unencrypted
    NVMe Device : /dev/nvme3n1 (BTOC0440005E058A)
                    Encryption(Ability|Status): None|Unencrypted

 
       Platform : Intel(R) Rapid Storage Technology
        Version : 22.0.0.5851
        License : Premium
    RAID Levels : raid0 raid1 raid5 raid10
    Chunk Sizes : 4k 8k 16k 32k 64k 128k
    2TB volumes : supported
      2TB disks : supported
      Max Disks : 32
    Max Volumes : 2 per array, 4 per controller
3rd party NVMe : supported
I/O Controller : /sys/devices/pci0000:00/0000:00:0e.0 (VMD)
    NVMe Device : /dev/nvme2n1 (PHLJ911200MT1P0FGN)
                    Encryption(Ability|Status): None|Unencrypted
    NVMe Device : /dev/nvme0n1 (PHLE7082001N2P0DGN)
                    Encryption(Ability|Status): None|Unencrypted

On same setup after the patch:

# mdadm --detail-p
mdadm: imsm capabilities not found for controller: /sys/devices/pci0000:80/0000:80:17.0 (type SATA) 
       Platform : Intel(R) Rapid Storage Technology
        Version : 22.0.0.5851
        License : Premium
    RAID Levels : raid0 raid1 raid5 raid10
    Chunk Sizes : 4k 8k 16k 32k 64k 128k
    2TB volumes : supported
      2TB disks : supported
      Max Disks : 32
    Max Volumes : 2 per array, 4 per controller
 3rd party NVMe : supported
 I/O Controller : /sys/devices/pci0000:00/0000:00:0e.0 (VMD)
 NVMe under VMD : /dev/nvme0n1 (253925805793)
                    Encryption(Ability|Status): SED|Unencrypted
 NVMe under VMD : /dev/nvme3n1 (PHAC112301EF3P8AGN)
                    Encryption(Ability|Status): None|Unencrypted
 NVMe under VMD : /dev/nvme6n1 (BTLN8430055B1P6AGN)
                    Encryption(Ability|Status): None|Unencrypted
 NVMe under VMD : /dev/nvme2n1 (BTLF731607DC1P0GGN)
                    Encryption(Ability|Status): None|Unencrypted
 NVMe under VMD : /dev/nvme5n1 (S7YDNS0L306079J)
                    Encryption(Ability|Status): SED|Unencrypted
 NVMe under VMD : /dev/nvme1n1 (S7YDNS0L101881W)
                    Encryption(Ability|Status): SED|Unencrypted
 NVMe under VMD : /dev/nvme4n1 (PHLJ911200MT1P0FGN)
                    Encryption(Ability|Status): None|Unencrypted

mtkaczyk · 2026-06-18T07:34:46Z

Thanks, for the patch, here some initial nits:

please attach example output to the commit or PR description.
please fix review failures.

This new PCH domain might not be implemented yet but
this patch should be backwards compatible.

Do it mean that the feature is not yet available in upstream kernel?

gardzinx · 2026-06-18T07:39:07Z

Thanks, for the patch, here some initial nits:
* please attach example output to the commit or PR description.

* please fix review failures.
This new PCH domain might not be implemented yet but
this patch should be backwards compatible.

Do it mean that the feature is not yet available in upstream kernel?

Yes, this feature is not yet implemented, it is going through review process at the moment.

mwilck · 2026-06-18T07:41:56Z

I/O Controller : /sys/devices/pci0000:00/0000:00:0e.0 (VMD)
I/O Controller : /sys/devices/pci0000:00/0000:00:0e.0 (VMD)

Exactly the same IO controller, twice? That looks wrong. Please explain.

gardzinx · 2026-06-18T07:52:42Z

I/O Controller : /sys/devices/pci0000:00/0000:00:0e.0 (VMD)
I/O Controller : /sys/devices/pci0000:00/0000:00:0e.0 (VMD)

Exactly the same IO controller, twice? That looks wrong. Please explain.

Yes, the same controller is handling both devices behind PCH and CPU and it was my idea to somehow group those devices. I was also considering solution that would avoid creating separate node for PCH domain and thus list all devices in one block. Should I explore more towards that end?

mwilck · 2026-06-18T08:11:19Z

I don't fully understand what's wrong with output "before this patch". I also don't fully understand the topology. IIUC some drives are directly attached tot he CPU, and others are attached to the South Bridge. Both are handled by the same controller device, though. IMO it doesn't matter much to users how a drive is connected (is there a noticeable performance difference?). I'd probably print the controller capabilities, and below it, list all drives that this controller manages.

Just my €0.02, like I said I don't fully understand the architecture.

gardzinx · 2026-06-18T08:15:04Z

I don't fully understand what's wrong with output "before this patch". I also don't fully understand the topology. IIUC some drives are directly attached tot he CPU, and others are attached to the South Bridge. Both are handled by the same controller device, though. IMO it doesn't matter much to users how a drive is connected (is there a noticeable performance difference?). I'd probably print the controller capabilities, and below it, list all drives that this controller manages.

Just my €0.02, like I said I don't fully understand the architecture.

The main problem is that before this patch, mdadm doesn't list drives as behind VMD when they are connected to PCH line. Instead there is whole new driver block 'Platform : Intel(R) Rapid Storage Technology enterprise' made.

mtkaczyk · 2026-06-18T09:49:09Z

Yes, the same controller is handling both devices behind PCH and CPU and it was my idea to somehow group those devices. I was also considering solution that would avoid creating separate node for PCH domain and thus list all devices in one block. Should I explore more towards that end?

Yes, please. Simply, printing the same "I/O Controller:" twice looks like a bug.

The main problem is that before this patch, mdadm doesn't list drives as behind VMD when they are connected to PCH line. Instead there is whole new driver block 'Platform : Intel(R) Rapid Storage Technology enterprise' made.

The problem with this 'Intel(R) Rapid Storage Technology enterprise' is probably limited to Intel drives that are no longer produced and sold. Please double check. For other drives, simply they are not listed.

Yes, this feature is not yet implemented, it is going through review process at the moment.

Well, then we cannot merge the feature in madm after it is merged in kernel. Experience tells that kernel folks may change their mind very last moment. Please notify us when it is merged.

mwilck · 2026-06-18T11:14:46Z

Well, then we cannot merge the feature in madm after it is merged in kernel.

Would the feature, if merged here, cause misbhehavior if it isn't merged in the kernel? If yes, which?

gardzinx · 2026-06-18T11:19:45Z

Well, then we cannot merge the feature in madm after it is merged in kernel.

Would the feature, if merged here, cause misbhehavior if it isn't merged in the kernel? If yes, which?

From my testing, this change should not change anything in current behavior. There is a check if there exists path in the controller for 'domain1' (which is not yet implemented and is affecting only certain platforms). If 'domain1' exists, it will create additional vmd node for it. The rest of changes are minor tweaks for places that use hardcoded path to 'domain' so its parameter instead.

mtkaczyk · 2026-06-23T05:45:50Z

Would the feature, if merged here, cause misbhehavior if it isn't merged in the kernel? If yes, which?

From my testing, this change should not change anything in current behavior. There is a check if there exists path in the controller for 'domain1' (which is not yet implemented and is affecting only certain platforms). If 'domain1' exists, it will create additional vmd node for it. The rest of changes are minor tweaks for places that use hardcoded path to 'domain' so its parameter instead.

@mwilck

Yes, it is expand of the functionality to follow domains not sole domain. Should have no negative consequences.

My concerns are about "domain1", maybe folks will decide to change the naming so we will end with never-used code. But still, if you think that it is safe to merge it earlier and eventually fix later- I have no strong objections just following general policy here.

gardzinx · 2026-06-26T11:24:29Z

Pushed a change where there is no more separate vmd node for PCH to avoid controller lame duplication during print - instead I added path to pch if it exists as a separate field in sys_dev struct. Modified output in description to reflect the change.

VMD drives behind PCH need to be handled the same way as the drives behind CPU - detect if PCH exists and handle them just like for CPU. This new PCH domain might not be implemented yet but this patch should be backwards compatible. Signed-off-by: Gardzinski, RomanX <romanx.gardzinski@intel.com>

mtkaczyk

Hey,
see comments.

mtkaczyk · 2026-06-30T09:30:34Z

 	enum sys_dev_type type;
 	char *path;
+	char *path_pch;
 	char *pci_id;


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.

gardzinx force-pushed the platform-intel/vmd_for_pch_devices branch 2 times, most recently from 5a7eeca to a13e77c Compare June 18, 2026 07:45

gardzinx force-pushed the platform-intel/vmd_for_pch_devices branch 2 times, most recently from 7fee7d5 to ccd406a Compare June 26, 2026 11:22

gardzinx force-pushed the platform-intel/vmd_for_pch_devices branch from ccd406a to eacd086 Compare June 26, 2026 11:31

mtkaczyk requested changes Jun 30, 2026

View reviewed changes

Uh oh!

Conversation

gardzinx commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtkaczyk commented Jun 18, 2026

Uh oh!

gardzinx commented Jun 18, 2026

Uh oh!

mwilck commented Jun 18, 2026

Uh oh!

gardzinx commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mwilck commented Jun 18, 2026

Uh oh!

gardzinx commented Jun 18, 2026

Uh oh!

mtkaczyk commented Jun 18, 2026

Uh oh!

mwilck commented Jun 18, 2026

Uh oh!

gardzinx commented Jun 18, 2026

Uh oh!

mtkaczyk commented Jun 23, 2026

Uh oh!

gardzinx commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtkaczyk left a comment

Choose a reason for hiding this comment

Uh oh!

mtkaczyk Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gardzinx commented Jun 18, 2026 •

edited

Loading

gardzinx commented Jun 18, 2026 •

edited

Loading

gardzinx commented Jun 26, 2026 •

edited

Loading