s390x: Add AP Attach/Detach test

Now that we have propper AP device support add a
unit test for testing the correct Attach/Detach of AP devices.

Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>

src/runtime/pkg/device/drivers/utils.go

@@ -111,7 +111,7 @@ func GetVFIODeviceType(deviceFilePath string) (config.VFIODeviceType, error) {
 		return config.VFIODeviceErrorType, err
 	}
 
-	if strings.HasPrefix(deviceSysfsDev, vfioAPSysfsDir) {
+	if strings.Contains(deviceSysfsDev, vfioAPSysfsDir) {
 		return config.VFIOAPDeviceMediatedType, nil
 	}
 

src/runtime/pkg/device/drivers/vfio.go

@@ -69,6 +69,7 @@ func (device *VFIODevice) Attach(ctx context.Context, devReceiver api.DeviceRece
 	if err != nil {
 		return err
 	}
+
 	for _, vfio := range device.VfioDevs {
 		// If vfio.Port is not set we bail out, users should set
 		// explicitly the port in the config file

src/runtime/pkg/device/manager/manager_test.go

@@ -90,6 +90,100 @@ func TestNewDevice(t *testing.T) {
 	assert.Equal(t, vfioDev.DeviceInfo.GID, uint32(2))
 }
 
+func TestAttachVFIOAPDevice(t *testing.T) {
+
+	var err error
+	var ok bool
+
+	dm := &deviceManager{
+		devices: make(map[string]api.Device),
+	}
+
+	tmpDir := t.TempDir()
+	// sys/devices/vfio_ap/matrix/f94290f8-78ac-45fb-bb22-e55e519fa64f
+	testSysfsAP := "/sys/devices/vfio_ap/"
+	testDeviceAP := "f94290f8-78ac-45fb-bb22-e55e519fa64f"
+	testVFIOGroup := "42"
+
+	matrixDir := filepath.Join(tmpDir, testSysfsAP, "matrix")
+	err = os.MkdirAll(matrixDir, dirMode)
+	assert.Nil(t, err)
+
+	deviceAPFile := filepath.Join(matrixDir, testDeviceAP)
+	err = os.MkdirAll(deviceAPFile, dirMode)
+	assert.Nil(t, err)
+
+	matrixDeviceAPFile := filepath.Join(deviceAPFile, "matrix")
+	_, err = os.Create(matrixDeviceAPFile)
+	assert.Nil(t, err)
+	// create AP devices in the matrix file
+	APDevices := []byte("05.001f\n")
+	err = os.WriteFile(matrixDeviceAPFile, APDevices, 0644)
+	assert.Nil(t, err)
+
+	devicesVFIOGroupDir := filepath.Join(tmpDir, testVFIOGroup, "devices")
+	err = os.MkdirAll(devicesVFIOGroupDir, dirMode)
+	assert.Nil(t, err)
+
+	deviceAPSymlink := filepath.Join(devicesVFIOGroupDir, testDeviceAP)
+	err = os.Symlink(deviceAPFile, deviceAPSymlink)
+	assert.Nil(t, err)
+
+	savedIOMMUPath := config.SysIOMMUGroupPath
+	config.SysIOMMUGroupPath = tmpDir
+
+	savedSysBusPciDevicesPath := config.SysBusPciDevicesPath
+	config.SysBusPciDevicesPath = devicesVFIOGroupDir
+
+	defer func() {
+		config.SysIOMMUGroupPath = savedIOMMUPath
+		config.SysBusPciDevicesPath = savedSysBusPciDevicesPath
+	}()
+
+	path := filepath.Join(vfioPath, testVFIOGroup)
+	deviceInfo := config.DeviceInfo{
+		HostPath:      path,
+		ContainerPath: path,
+		DevType:       "c",
+		ColdPlug:      false,
+		Port:          config.RootPort,
+	}
+
+	device, err := dm.NewDevice(deviceInfo)
+	assert.Nil(t, err)
+	_, ok = device.(*drivers.VFIODevice)
+	assert.True(t, ok)
+
+	devReceiver := &api.MockDeviceReceiver{}
+	err = device.Attach(context.Background(), devReceiver)
+	assert.Nil(t, err)
+
+	err = device.Detach(context.Background(), devReceiver)
+	assert.Nil(t, err)
+
+	// If we omit the port setting we should fail
+	failDm := &deviceManager{
+		devices: make(map[string]api.Device),
+	}
+
+	failDeviceInfo := config.DeviceInfo{
+		HostPath:      path,
+		ContainerPath: path,
+		DevType:       "c",
+		ColdPlug:      false,
+	}
+
+	failDevice, err := failDm.NewDevice(failDeviceInfo)
+	assert.Nil(t, err)
+	_, ok = failDevice.(*drivers.VFIODevice)
+	assert.True(t, ok)
+
+	failDevReceiver := &api.MockDeviceReceiver{}
+	err = failDevice.Attach(context.Background(), failDevReceiver)
+	assert.Error(t, err)
+
+}
+
 func TestAttachVFIODevice(t *testing.T) {
 	dm := &deviceManager{
 		blockDriver: config.VirtioBlock,

src/runtime/pkg/katautils/config.go

@@ -22,6 +22,7 @@ import (
 	govmmQemu "github.com/kata-containers/kata-containers/src/runtime/pkg/govmm/qemu"
 	"github.com/kata-containers/kata-containers/src/runtime/pkg/katautils/katatrace"
 	"github.com/kata-containers/kata-containers/src/runtime/pkg/oci"
+	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers"
 	vc "github.com/kata-containers/kata-containers/src/runtime/virtcontainers"
 	exp "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/experimental"
 	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils"
@@ -1667,7 +1668,8 @@ func checkConfig(config oci.RuntimeConfig) error {
 	hotPlugVFIO := config.HypervisorConfig.HotPlugVFIO
 	coldPlugVFIO := config.HypervisorConfig.ColdPlugVFIO
 	machineType := config.HypervisorConfig.HypervisorMachineType
-	if err := checkPCIeConfig(coldPlugVFIO, hotPlugVFIO, machineType); err != nil {
+	hypervisorType := config.HypervisorType
+	if err := checkPCIeConfig(coldPlugVFIO, hotPlugVFIO, machineType, hypervisorType); err != nil {
 		return err
 	}
 
@@ -1677,16 +1679,20 @@ func checkConfig(config oci.RuntimeConfig) error {
 // checkPCIeConfig ensures the PCIe configuration is valid.
 // Only allow one of the following settings for cold-plug:
 // no-port, root-port, switch-port
-func checkPCIeConfig(coldPlug config.PCIePort, hotPlug config.PCIePort, machineType string) error {
+func checkPCIeConfig(coldPlug config.PCIePort, hotPlug config.PCIePort, machineType string, hypervisorType virtcontainers.HypervisorType) error {
-	// Currently only QEMU q35,virt support advanced PCIe topologies
+	if hypervisorType != virtcontainers.QemuHypervisor {
-	// firecracker, dragonball do not have right now any PCIe support
+		kataUtilsLogger.Warn("Advanced PCIe Topology only available for QEMU hypervisor, ignoring hot(cold)_vfio_port setting")
+		return nil
+	}
+
 	if coldPlug != config.NoPort && hotPlug != config.NoPort {
 		return fmt.Errorf("invalid hot-plug=%s and cold-plug=%s settings, only one of them can be set", coldPlug, hotPlug)
 	}
 	if coldPlug == config.NoPort && hotPlug == config.NoPort {
 		return nil
 	}
-
+	// Currently only QEMU q35,virt support advanced PCIe topologies
+	// firecracker, dragonball do not have right now any PCIe support
 	if machineType != "q35" && machineType != "virt" {
 		return nil
 	}

src/runtime/virtcontainers/qemu.go

@@ -775,6 +775,7 @@ func (q *qemu) createPCIeTopology(qemuConfig *govmmQemu.Config, hypervisorConfig
 		if err != nil {
 			return fmt.Errorf("Cannot get host path for device: %v err: %v", dev, err)
 		}
+
 		devicesPerIOMMUGroup, err := drivers.GetAllVFIODevicesFromIOMMUGroup(dev)
 		if err != nil {
 			return fmt.Errorf("Cannot get all VFIO devices from IOMMU group with device: %v err: %v", dev, err)

src/runtime/virtcontainers/sandbox.go

@@ -613,6 +613,36 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 		return nil, err
 	}
 
+	coldPlugVFIO, err := s.coldOrHotPlugVFIO(&sandboxConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	// store doesn't require hypervisor to be stored immediately
+	if err = s.hypervisor.CreateVM(ctx, s.id, s.network, &sandboxConfig.HypervisorConfig); err != nil {
+		return nil, err
+	}
+
+	if s.disableVMShutdown, err = s.agent.init(ctx, s, sandboxConfig.AgentConfig); err != nil {
+		return nil, err
+	}
+
+	if !coldPlugVFIO {
+		return s, nil
+	}
+
+	for _, dev := range sandboxConfig.HypervisorConfig.VFIODevices {
+		s.Logger().Info("cold-plug device: ", dev)
+		_, err := s.AddDevice(ctx, dev)
+		if err != nil {
+			s.Logger().WithError(err).Debug("Cannot cold-plug add device")
+			return nil, err
+		}
+	}
+	return s, nil
+}
+
+func (s *Sandbox) coldOrHotPlugVFIO(sandboxConfig *SandboxConfig) (bool, error) {
 	// If we have a confidential guest we need to cold-plug the PCIe VFIO devices
 	// until we have TDISP/IDE PCIe support.
 	coldPlugVFIO := (sandboxConfig.HypervisorConfig.ColdPlugVFIO != config.NoPort)
@@ -620,8 +650,8 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 	// the correct amount of ports to reserve for the hypervisor.
 	hotPlugVFIO := (sandboxConfig.HypervisorConfig.HotPlugVFIO != config.NoPort)
 
-	modeGK := (sandboxConfig.VfioMode == config.VFIOModeGuestKernel)
+	modeIsGK := (sandboxConfig.VfioMode == config.VFIOModeGuestKernel)
-	modeVFIO := (sandboxConfig.VfioMode == config.VFIOModeVFIO)
+	modeIsVFIO := (sandboxConfig.VfioMode == config.VFIOModeVFIO)
 
 	var vfioDevices []config.DeviceInfo
 	// vhost-user-block device is a PCIe device in Virt, keep track of it
@@ -639,7 +669,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 			isVFIOControlDevice := deviceManager.IsVFIOControlDevice(device.ContainerPath)
 			// vfio_mode=vfio needs the VFIO control device add it to the list
 			// of devices to be added to the VM.
-			if modeVFIO && isVFIOControlDevice {
+			if modeIsVFIO && isVFIOControlDevice && !hotPlugVFIO {
 				vfioDevices = append(vfioDevices, device)
 			}
 
@@ -656,7 +686,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 				// We need to remove the devices marked for cold-plug
 				// otherwise at the container level the kata-agent
 				// will try to hot-plug them.
-				if modeGK {
+				if modeIsGK {
 					sandboxConfig.Containers[cnt].DeviceInfos[dev].ID = "remove-we-are-cold-plugging"
 				}
 			}
@@ -668,34 +698,12 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 			}
 		}
 		sandboxConfig.Containers[cnt].DeviceInfos = filteredDevices
-
 	}
 
 	sandboxConfig.HypervisorConfig.VFIODevices = vfioDevices
 	sandboxConfig.HypervisorConfig.VhostUserBlkDevices = vhostUserBlkDevices
 
-	// store doesn't require hypervisor to be stored immediately
+	return coldPlugVFIO, nil
-	if err = s.hypervisor.CreateVM(ctx, s.id, s.network, &sandboxConfig.HypervisorConfig); err != nil {
-		return nil, err
-	}
-
-	if s.disableVMShutdown, err = s.agent.init(ctx, s, sandboxConfig.AgentConfig); err != nil {
-		return nil, err
-	}
-
-	if !coldPlugVFIO {
-		return s, nil
-	}
-
-	for _, dev := range vfioDevices {
-		s.Logger().Info("cold-plug device: ", dev)
-		_, err := s.AddDevice(ctx, dev)
-		if err != nil {
-			s.Logger().WithError(err).Debug("Cannot cold-plug add device")
-			return nil, err
-		}
-	}
-	return s, nil
 }
 
 func (s *Sandbox) createResourceController() error {