Skip to content

Commit 416679f

Browse files
vishesh92phiea
vishesh92
and
phiea
authored
Fix domain parsing for GPU & add Display controller in the supported PCI class (#12981)
* Fix domain parsing for GPU * Add Display controller to GPU class check this adds support for the amd instinct mi2xx accelorator crards in the discovery script. Co-authored-by: Piet Braat <piet@phiea.nl>
1 parent b1bc538 commit 416679f

File tree

3 files changed

+327
-63
lines changed

3 files changed

+327
-63
lines changed

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtGpuDef.java‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -79,28 +79,47 @@ private void generatePciXml(StringBuilder gpuBuilder) {
7979
gpuBuilder.append(" <driver name='vfio'/>\n");
8080
gpuBuilder.append(" <source>\n");
8181

82-
// Parse the bus address (e.g., 00:02.0) into domain, bus, slot, function
83-
String domain = "0x0000";
84-
String bus = "0x00";
85-
String slot = "0x00";
86-
String function = "0x0";
82+
// Parse the bus address into domain, bus, slot, function. Two input formats are accepted:
83+
// - "dddd:bb:ss.f" full PCI address with domain (e.g. 0000:00:02.0)
84+
// - "bb:ss.f" legacy short BDF; domain defaults to 0000
85+
// Each segment is parsed as a hex integer and formatted with fixed widths
86+
// (domain: 4 hex digits, bus/slot: 2 hex digits, function: 1 hex digit) to
87+
// produce canonical libvirt XML values regardless of input casing or padding.
88+
int domainVal = 0, busVal = 0, slotVal = 0, funcVal = 0;
8789

8890
if (busAddress != null && !busAddress.isEmpty()) {
8991
String[] parts = busAddress.split(":");
90-
if (parts.length > 1) {
91-
bus = "0x" + parts[0];
92-
String[] slotFunctionParts = parts[1].split("\\.");
93-
if (slotFunctionParts.length > 0) {
94-
slot = "0x" + slotFunctionParts[0];
95-
if (slotFunctionParts.length > 1) {
96-
function = "0x" + slotFunctionParts[1].trim();
97-
}
92+
try {
93+
String slotFunction;
94+
if (parts.length == 3) {
95+
domainVal = Integer.parseInt(parts[0], 16);
96+
busVal = Integer.parseInt(parts[1], 16);
97+
slotFunction = parts[2];
98+
} else if (parts.length == 2) {
99+
busVal = Integer.parseInt(parts[0], 16);
100+
slotFunction = parts[1];
101+
} else {
102+
throw new IllegalArgumentException("Invalid PCI bus address format: '" + busAddress + "'");
98103
}
104+
String[] sf = slotFunction.split("\\.");
105+
if (sf.length == 2) {
106+
slotVal = Integer.parseInt(sf[0], 16);
107+
funcVal = Integer.parseInt(sf[1].trim(), 16);
108+
} else {
109+
throw new IllegalArgumentException("Invalid PCI bus address format: '" + busAddress + "'");
110+
}
111+
} catch (NumberFormatException e) {
112+
throw new IllegalArgumentException("Invalid PCI bus address format: '" + busAddress + "'", e);
99113
}
100114
}
101115

116+
String domain = String.format("0x%04x", domainVal);
117+
String bus = String.format("0x%02x", busVal);
118+
String slot = String.format("0x%02x", slotVal);
119+
String function = String.format("0x%x", funcVal);
120+
102121
gpuBuilder.append(" <address domain='").append(domain).append("' bus='").append(bus).append("' slot='")
103-
.append(slot).append("' function='").append(function.trim()).append("'/>\n");
122+
.append(slot).append("' function='").append(function).append("'/>\n");
104123
gpuBuilder.append(" </source>\n");
105124
gpuBuilder.append("</hostdev>\n");
106125
}

‎plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtGpuDefTest.java‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,145 @@ public void testGpuDef_withComplexPciAddress() {
115115
assertTrue(gpuXml.contains("</hostdev>"));
116116
}
117117

118+
@Test
119+
public void testGpuDef_withFullPciAddressDomainZero() {
120+
LibvirtGpuDef gpuDef = new LibvirtGpuDef();
121+
VgpuTypesInfo pciGpuInfo = new VgpuTypesInfo(
122+
GpuDevice.DeviceType.PCI,
123+
"passthrough",
124+
"passthrough",
125+
"0000:00:02.0",
126+
"10de",
127+
"NVIDIA Corporation",
128+
"1b38",
129+
"Tesla T4"
130+
);
131+
gpuDef.defGpu(pciGpuInfo);
132+
133+
String gpuXml = gpuDef.toString();
134+
135+
assertTrue(gpuXml.contains("<address domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>"));
136+
}
137+
138+
@Test
139+
public void testGpuDef_withFullPciAddressNonZeroDomain() {
140+
LibvirtGpuDef gpuDef = new LibvirtGpuDef();
141+
VgpuTypesInfo pciGpuInfo = new VgpuTypesInfo(
142+
GpuDevice.DeviceType.PCI,
143+
"passthrough",
144+
"passthrough",
145+
"0001:65:00.0",
146+
"10de",
147+
"NVIDIA Corporation",
148+
"1b38",
149+
"Tesla T4"
150+
);
151+
gpuDef.defGpu(pciGpuInfo);
152+
153+
String gpuXml = gpuDef.toString();
154+
155+
assertTrue(gpuXml.contains("<address domain='0x0001' bus='0x65' slot='0x00' function='0x0'/>"));
156+
}
157+
158+
@Test
159+
public void testGpuDef_withNvidiaStyleEightDigitDomain() {
160+
// nvidia-smi reports PCI addresses with an 8-digit domain (e.g. "00000001:af:00.1").
161+
// generatePciXml must normalize it to the canonical 4-digit form "0x0001".
162+
LibvirtGpuDef gpuDef = new LibvirtGpuDef();
163+
VgpuTypesInfo pciGpuInfo = new VgpuTypesInfo(
164+
GpuDevice.DeviceType.PCI,
165+
"passthrough",
166+
"passthrough",
167+
"00000001:af:00.1",
168+
"10de",
169+
"NVIDIA Corporation",
170+
"1b38",
171+
"Tesla T4"
172+
);
173+
gpuDef.defGpu(pciGpuInfo);
174+
175+
String gpuXml = gpuDef.toString();
176+
177+
assertTrue(gpuXml.contains("<address domain='0x0001' bus='0xaf' slot='0x00' function='0x1'/>"));
178+
}
179+
180+
@Test
181+
public void testGpuDef_withFullPciAddressVfNonZeroDomain() {
182+
LibvirtGpuDef gpuDef = new LibvirtGpuDef();
183+
VgpuTypesInfo vfGpuInfo = new VgpuTypesInfo(
184+
GpuDevice.DeviceType.PCI,
185+
"VF-Profile",
186+
"VF-Profile",
187+
"0002:81:00.3",
188+
"10de",
189+
"NVIDIA Corporation",
190+
"1eb8",
191+
"Tesla T4"
192+
);
193+
gpuDef.defGpu(vfGpuInfo);
194+
195+
String gpuXml = gpuDef.toString();
196+
197+
// Non-passthrough NVIDIA VFs should be unmanaged
198+
assertTrue(gpuXml.contains("<hostdev mode='subsystem' type='pci' managed='no' display='off'>"));
199+
assertTrue(gpuXml.contains("<address domain='0x0002' bus='0x81' slot='0x00' function='0x3'/>"));
200+
}
201+
202+
@Test
203+
public void testGpuDef_withLegacyShortBdfDefaultsDomainToZero() {
204+
// Backward compatibility: short BDF with no domain segment must still
205+
// produce a valid libvirt address with domain 0x0000.
206+
LibvirtGpuDef gpuDef = new LibvirtGpuDef();
207+
VgpuTypesInfo pciGpuInfo = new VgpuTypesInfo(
208+
GpuDevice.DeviceType.PCI,
209+
"passthrough",
210+
"passthrough",
211+
"af:00.0",
212+
"10de",
213+
"NVIDIA Corporation",
214+
"1b38",
215+
"Tesla T4"
216+
);
217+
gpuDef.defGpu(pciGpuInfo);
218+
219+
String gpuXml = gpuDef.toString();
220+
221+
assertTrue(gpuXml.contains("<address domain='0x0000' bus='0xaf' slot='0x00' function='0x0'/>"));
222+
}
223+
224+
@Test
225+
public void testGpuDef_withInvalidBusAddressThrows() {
226+
String[] invalidAddresses = {
227+
"notahex:00.0", // non-hex bus
228+
"gg:00:02.0", // non-hex domain
229+
"00:02:03:04", // too many colon-separated parts
230+
"00", // missing slot/function
231+
"00:02", // missing function (no dot)
232+
"00:02.0.1", // extra dot in ss.f
233+
};
234+
for (String addr : invalidAddresses) {
235+
LibvirtGpuDef gpuDef = new LibvirtGpuDef();
236+
VgpuTypesInfo info = new VgpuTypesInfo(
237+
GpuDevice.DeviceType.PCI,
238+
"passthrough",
239+
"passthrough",
240+
addr,
241+
"10de",
242+
"NVIDIA Corporation",
243+
"1b38",
244+
"Tesla T4"
245+
);
246+
gpuDef.defGpu(info);
247+
try {
248+
String ignored = gpuDef.toString();
249+
fail("Expected IllegalArgumentException for address: " + addr + " but got: " + ignored);
250+
} catch (IllegalArgumentException e) {
251+
assertTrue("Exception message should contain the bad address",
252+
e.getMessage().contains(addr));
253+
}
254+
}
255+
}
256+
118257
@Test
119258
public void testGpuDef_withNullDeviceType() {
120259
LibvirtGpuDef gpuDef = new LibvirtGpuDef();

0 commit comments

Comments
 (0)