From 1fc715bc6526f53057e3bc5f0bc118907e4f089a Mon Sep 17 00:00:00 2001 From: Zvonko Kaiser Date: Fri, 21 Jul 2023 07:44:36 +0000 Subject: [PATCH] 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 --- src/runtime/pkg/device/drivers/utils.go | 2 +- src/runtime/pkg/device/drivers/vfio.go | 1 + .../pkg/device/manager/manager_test.go | 94 +++++++++++++++++++ src/runtime/pkg/katautils/config.go | 16 +++- src/runtime/virtcontainers/qemu.go | 1 + src/runtime/virtcontainers/sandbox.go | 62 ++++++------ 6 files changed, 143 insertions(+), 33 deletions(-) diff --git a/src/runtime/pkg/device/drivers/utils.go b/src/runtime/pkg/device/drivers/utils.go index 4fa0fdd0e2..5f76bff48e 100644 --- a/src/runtime/pkg/device/drivers/utils.go +++ b/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 } diff --git a/src/runtime/pkg/device/drivers/vfio.go b/src/runtime/pkg/device/drivers/vfio.go index 7edd5b3c6e..feec9c4482 100644 --- a/src/runtime/pkg/device/drivers/vfio.go +++ b/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 diff --git a/src/runtime/pkg/device/manager/manager_test.go b/src/runtime/pkg/device/manager/manager_test.go index bcd321477f..c08cb66ab4 100644 --- a/src/runtime/pkg/device/manager/manager_test.go +++ b/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, diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index cb59b4f167..f577df5263 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/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 { - // Currently only QEMU q35,virt support advanced PCIe topologies - // firecracker, dragonball do not have right now any PCIe support +func checkPCIeConfig(coldPlug config.PCIePort, hotPlug config.PCIePort, machineType string, hypervisorType virtcontainers.HypervisorType) error { + if hypervisorType != virtcontainers.QemuHypervisor { + 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 } diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 1a74944baa..57fa036f61 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/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) diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 3507807323..a4d7e07302 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/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) - modeVFIO := (sandboxConfig.VfioMode == config.VFIOModeVFIO) + modeIsGK := (sandboxConfig.VfioMode == config.VFIOModeGuestKernel) + 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 - 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 + return coldPlugVFIO, nil } func (s *Sandbox) createResourceController() error {