pci-placement: Isolate pcie-expander-bus per IOMMU device when sharing a NUMA node

On a 2-superchip GB200, each Grace CPU has 2 Blackwell GPUs connected
via NVLink-C2C that share the same sysfs NUMA node as the CPU socket:

  GPU 1 (0008:01:00.0): NUMA 0 (Grace CPU kubevirt#1)
  GPU 2 (0009:01:00.0): NUMA 0 (Grace CPU kubevirt#1)
  GPU 3 (0018:01:00.0): NUMA 1 (Grace CPU kubevirt#2)
  GPU 4 (0019:01:00.0): NUMA 1 (Grace CPU kubevirt#2)

VEP 115's per-NUMA expander bus placement puts both GPUs on the same
pcie-expander-bus, which means they share a single IOMMU device
created by placeDevice. On ARM64 Grace systems this shared SMMUv3
causes CMD_SYNC timeout and tegra241_cmdqv errors in the guest,
preventing multi-GPU initialization.

When multiple IOMMU devices (marked with ACPI NodeSet "tofill") share
a NUMA node, each now gets its own pcie-expander-bus controller. A
single IOMMU device on a NUMA node continues to use the shared
per-NUMA expander bus. Non-IOMMU devices always share a
pcie-expander-bus per NUMA node.

Ref: VOYAGER-707

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Lee Yarwood <lyarwood@redhat.com>

Author: lyarwood

pkg/virt-launcher/virtwrap/converter/pci-placement.go

@@ -188,6 +188,7 @@ type expanderBusAssigner struct {
 	controllerIndex  uint32
 	controllerCount  uint32
 	topologyMap      map[uint32]*numaAwareTopology
+	deviceTopologies []*numaAwareTopology
 	devices          map[string]*api.HostDevice
 	devicesNUMANodes map[string]uint32
 
@@ -329,6 +330,17 @@ func (a *expanderBusAssigner) getNumaAwareTopology(numaKey uint32) *numaAwareTop
 	return topology
 }
 
+// createDeviceTopology creates a dedicated topology for a single device,
+// with its own expander bus tagged to the specified NUMA node.
+func (a *expanderBusAssigner) createDeviceTopology(numaKey uint32) *numaAwareTopology {
+	topology := &numaAwareTopology{
+		expanderBus:               a.createController(api.ControllerModelPCIeExpanderBus, "", 0, &numaKey),
+		addressPerDeviceSourcePCI: make(map[string]*api.Address),
+	}
+	a.deviceTopologies = append(a.deviceTopologies, topology)
+	return topology
+}
+
 // addRootPort creates a PCIe root port and adds it to the topology.
 func (a *expanderBusAssigner) addRootPort(topology *numaAwareTopology, parentBus string) *api.Controller {
 	slot := uint32(len(topology.rootPorts))
@@ -397,34 +409,58 @@ func (a *expanderBusAssigner) placeDevice(topology *numaAwareTopology, device *a
 	return nil
 }
 
-// buildTopology groups devices by NUMA node by using a pcie-expander-bus per
-// NUMA node. Within a pcie-expander-bus one pcie-root-port per device is created.
-// Each device is then placed behind its respective root port.
-//
-// pcie-expander-bus (one per NUMA node) -> pcie-root-port (one per device) -> device
+// buildTopology groups devices by NUMA node. When multiple IOMMU devices
+// (marked with ACPI NodeSet "tofill") share a NUMA node, each gets its own
+// pcie-expander-bus to avoid sharing a single IOMMU device. A single IOMMU
+// device on a NUMA node shares the per-NUMA expander bus. Non-IOMMU devices
+// always share a pcie-expander-bus per NUMA node. Within each expander bus,
+// one pcie-root-port per device is created.
 //
-// It modifies the topology per NUMA node in place by creating the necessary controllers
+// It modifies the topology in place by creating the necessary controllers
 // and updating the addresses of the devices.
 func (a *expanderBusAssigner) buildTopology() error {
 	numaDeviceGroups := a.groupDevicesByNUMA()
 
+	iommuCountPerNUMA := make(map[uint32]int)
+	for numaKey, devices := range numaDeviceGroups {
+		for _, device := range devices {
+			if device.ACPI != nil && device.ACPI.NodeSet == "tofill" {
+				iommuCountPerNUMA[numaKey]++
+			}
+		}
+	}
+
 	for numaKey, devices := range numaDeviceGroups {
-		topology := a.getNumaAwareTopology(numaKey)
+		needsPerDeviceIsolation := iommuCountPerNUMA[numaKey] > 1
 
 		for _, device := range devices {
+			isIOMMUDevice := device.ACPI != nil && device.ACPI.NodeSet == "tofill"
+
+			var topology *numaAwareTopology
+			if isIOMMUDevice && needsPerDeviceIsolation {
+				topology = a.createDeviceTopology(numaKey)
+			} else {
+				topology = a.getNumaAwareTopology(numaKey)
+			}
+
 			if err := a.placeDevice(topology, device); err != nil {
 				return fmt.Errorf("failed to place device %s: %w", hardware.PCIAddressToString(device.Source.Address), err)
 			}
-		}
 
-		// Set the busNr of the expander bus so that it has enough space for all
-		// its children. We start from 254 (1 expander bus + 1 root port, when one
-		// device is aligned with a NUMA node) and go downwards to leave space for
-		// system controllers and additional expander buses.
-		busNr := maxExpanderBusNr - a.controllerCount + 1
-		topology.expanderBus.Target.BusNr = ptr.To(busNr)
+			if isIOMMUDevice && needsPerDeviceIsolation {
+				busNr := maxExpanderBusNr - a.controllerCount + 1
+				topology.expanderBus.Target.BusNr = ptr.To(busNr)
+				a.lastAssignedBusNr = busNr
+			}
+		}
 
-		a.lastAssignedBusNr = busNr
+		// Set the busNr of the shared expander bus (if any devices used it)
+		// so that it has enough space for all its children.
+		if topology, exists := a.topologyMap[numaKey]; exists {
+			busNr := maxExpanderBusNr - a.controllerCount + 1
+			topology.expanderBus.Target.BusNr = ptr.To(busNr)
+			a.lastAssignedBusNr = busNr
+		}
 	}
 
 	return nil
@@ -440,7 +476,13 @@ func (a *expanderBusAssigner) PlaceNumaAlignedDevices() error {
 		return fmt.Errorf("failed to create PCIe topology with NUMA alignment: %w", err)
 	}
 
-	for _, topology := range a.topologyMap {
+	allTopologies := make([]*numaAwareTopology, 0, len(a.topologyMap)+len(a.deviceTopologies))
+	for _, t := range a.topologyMap {
+		allTopologies = append(allTopologies, t)
+	}
+	allTopologies = append(allTopologies, a.deviceTopologies...)
+
+	for _, topology := range allTopologies {
 		a.domainSpec.Devices.Controllers = append(a.domainSpec.Devices.Controllers, *topology.expanderBus)
 
 		for _, rootPort := range topology.rootPorts {
@@ -451,9 +493,6 @@ func (a *expanderBusAssigner) PlaceNumaAlignedDevices() error {
 			if device, exists := a.devices[sourceAddress]; exists {
 				device.Address = address
 			}
-			// If a device was not placed in the topology (e.g. missing vCPU
-			// affinity information), we leave it unmodified so that it can be
-			// placed by the root slot assigner.
 		}
 		if topology.iommuDev != nil {
 			a.domainSpec.Devices.IOMMU = append(a.domainSpec.Devices.IOMMU, *topology.iommuDev)

pkg/virt-launcher/virtwrap/converter/pci-placement_test.go

@@ -72,6 +72,12 @@ var _ = Describe("PCIe Expander Bus Assigner", func() {
 		}
 	}
 
+	createIOMMUPCIDevice := func(alias, bus string) api.HostDevice {
+		dev := createPCIDevice(alias, bus)
+		dev.ACPI = &api.ACPIHostDev{NodeSet: "tofill"}
+		return dev
+	}
+
 	createNonPCIDevice := func(deviceType string) api.HostDevice {
 		return api.HostDevice{
 			Type: deviceType,
@@ -312,9 +318,131 @@ var _ = Describe("PCIe Expander Bus Assigner", func() {
 				devices:             []api.HostDevice{createPCIDevice("device1", "0x01")},
 				expectedControllers: 0,
 			}),
+			Entry("places single IOMMU device with dedicated expander bus", devicePlacementTestCase{
+				name:                  "single IOMMU device",
+				devices:               []api.HostDevice{createIOMMUPCIDevice("gpu1", "0x01")},
+				expectedControllers:   2,
+				expectedExpanderBuses: 1,
+				expectedRootPorts:     1,
+			}),
+			Entry("places multiple IOMMU devices on same NUMA node with separate expander buses", devicePlacementTestCase{
+				name: "multiple IOMMU devices same NUMA",
+				devices: []api.HostDevice{
+					createIOMMUPCIDevice("gpu1", "0x01"),
+					createIOMMUPCIDevice("gpu2", "0x03"),
+				},
+				expectedControllers:   4,
+				expectedExpanderBuses: 2,
+				expectedRootPorts:     2,
+			}),
+			Entry("places IOMMU devices on different NUMA nodes with separate expander buses", devicePlacementTestCase{
+				name: "IOMMU devices on different NUMA nodes",
+				devices: []api.HostDevice{
+					createIOMMUPCIDevice("gpu_numa0", "0x01"),
+					createIOMMUPCIDevice("gpu_numa1", "0x02"),
+				},
+				expectedControllers:   4,
+				expectedExpanderBuses: 2,
+				expectedRootPorts:     2,
+			}),
 		)
 	})
 
+	Describe("IOMMU device topology isolation", func() {
+		var (
+			domainSpec *api.DomainSpec
+			iommuPCI   *iommupci.IommuPCI
+		)
+
+		BeforeEach(func() {
+			domainSpec = createDomainSpecWithNUMA(
+				[]api.NUMACell{{ID: "0", CPUs: "0-1"}, {ID: "1", CPUs: "2-3"}},
+				[]api.CPUTuneVCPUPin{{VCPU: 0, CPUSet: "0"}, {VCPU: 2, CPUSet: "4"}},
+			)
+			iommuPCI = iommupci.NewIommuPCI(runtime.GOARCH)
+		})
+
+		It("should create separate smmuv3 IOMMU devices for each GPU on the same NUMA node", func() {
+			domainSpec.Devices.HostDevices = []api.HostDevice{
+				createIOMMUPCIDevice("gpu1", "0x01"),
+				createIOMMUPCIDevice("gpu2", "0x03"),
+			}
+
+			err := PlacePCIDevicesWithNUMAAlignment(domainSpec, iommuPCI)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(domainSpec.Devices.IOMMU).To(HaveLen(2), "each GPU should have its own smmuv3")
+			Expect(domainSpec.Devices.IOMMU[0].Model).To(Equal("smmuv3"))
+			Expect(domainSpec.Devices.IOMMU[1].Model).To(Equal("smmuv3"))
+
+			Expect(domainSpec.Devices.IOMMU[0].Driver.PciBus).ToNot(Equal(domainSpec.Devices.IOMMU[1].Driver.PciBus),
+				"each smmuv3 should reference a different expander bus")
+		})
+
+		It("should assign non-overlapping bus numbers to per-device expander buses", func() {
+			domainSpec.Devices.HostDevices = []api.HostDevice{
+				createIOMMUPCIDevice("gpu1", "0x01"),
+				createIOMMUPCIDevice("gpu2", "0x03"),
+			}
+
+			err := PlacePCIDevicesWithNUMAAlignment(domainSpec, iommuPCI)
+			Expect(err).ToNot(HaveOccurred())
+
+			busNumbers := map[uint32]bool{}
+			for _, controller := range domainSpec.Devices.Controllers {
+				if controller.Model == api.ControllerModelPCIeExpanderBus {
+					Expect(controller.Target).ToNot(BeNil())
+					Expect(controller.Target.BusNr).ToNot(BeNil())
+					busNr := *controller.Target.BusNr
+					Expect(busNumbers[busNr]).To(BeFalse(), "bus number %d assigned twice", busNr)
+					busNumbers[busNr] = true
+				}
+			}
+			Expect(busNumbers).To(HaveLen(2))
+		})
+
+		It("should use shared per-NUMA expander bus when only one IOMMU device on a NUMA node", func() {
+			domainSpec.Devices.HostDevices = []api.HostDevice{
+				createIOMMUPCIDevice("gpu1", "0x01"),
+				createPCIDevice("nic1", "0x03"),
+			}
+
+			err := PlacePCIDevicesWithNUMAAlignment(domainSpec, iommuPCI)
+			Expect(err).ToNot(HaveOccurred())
+
+			expanderBuses := 0
+			for _, controller := range domainSpec.Devices.Controllers {
+				if controller.Model == api.ControllerModelPCIeExpanderBus {
+					expanderBuses++
+				}
+			}
+			Expect(expanderBuses).To(Equal(1), "single IOMMU device should share the per-NUMA expander bus")
+
+			Expect(domainSpec.Devices.IOMMU).To(HaveLen(1), "single IOMMU device should still get an smmuv3")
+			Expect(domainSpec.Devices.IOMMU[0].Model).To(Equal("smmuv3"))
+		})
+
+		It("should assign each GPU device to its own root port on separate expander buses", func() {
+			domainSpec.Devices.HostDevices = []api.HostDevice{
+				createIOMMUPCIDevice("gpu1", "0x01"),
+				createIOMMUPCIDevice("gpu2", "0x03"),
+			}
+
+			err := PlacePCIDevicesWithNUMAAlignment(domainSpec, iommuPCI)
+			Expect(err).ToNot(HaveOccurred())
+
+			deviceBuses := map[string]bool{}
+			for _, device := range domainSpec.Devices.HostDevices {
+				Expect(device.Address).ToNot(BeNil())
+				Expect(device.Address.Bus).ToNot(BeEmpty())
+				Expect(deviceBuses[device.Address.Bus]).To(BeFalse(),
+					"bus %s used by multiple devices", device.Address.Bus)
+				deviceBuses[device.Address.Bus] = true
+			}
+			Expect(deviceBuses).To(HaveLen(2))
+		})
+	})
+
 	Describe("PlacePCIDevicesWithNUMAAlignment", func() {
 		var (
 			domainSpec *api.DomainSpec