From 31e2925a9a529e890eb38a1f612325437c5cd882 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 16 Aug 2018 16:04:09 -0700 Subject: [PATCH] vfio: Add configuration to support VFIO hotplug on root bus We need this configuration due to a limitation in seabios firmware in handling hotplug for PCI devices with large BARS. Long term, this needs to be fixed in the firmware. Fixes #594 Signed-off-by: Archana Shinde --- Makefile | 4 ++++ cli/config.go | 5 ++++- cli/config/configuration.toml.in | 7 +++++++ cli/config_test.go | 14 ++++++++++++-- cli/kata-env_test.go | 2 ++ virtcontainers/hypervisor.go | 4 ++++ virtcontainers/qemu.go | 22 +++++++++++++++++----- 7 files changed, 50 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index c77ddf3657..fa01aa8ca0 100644 --- a/Makefile +++ b/Makefile @@ -130,6 +130,7 @@ DEFENABLESWAP := false DEFENABLEDEBUG := false DEFDISABLENESTINGCHECKS := false DEFMSIZE9P := 8192 +DEFHOTPLUGVFIOONROOTBUS := false SED = sed @@ -202,6 +203,7 @@ USER_VARS += DEFENABLESWAP USER_VARS += DEFENABLEDEBUG USER_VARS += DEFDISABLENESTINGCHECKS USER_VARS += DEFMSIZE9P +USER_VARS += DEFHOTPLUGVFIOONROOTBUS V = @ Q = $(V:1=) @@ -296,6 +298,7 @@ const defaultEnableSwap bool = $(DEFENABLESWAP) const defaultEnableDebug bool = $(DEFENABLEDEBUG) const defaultDisableNestingChecks bool = $(DEFDISABLENESTINGCHECKS) const defaultMsize9p uint32 = $(DEFMSIZE9P) +const defaultHotplugVFIOOnRootBus bool = $(DEFHOTPLUGVFIOONROOTBUS) // Default config file used by stateless systems. var defaultRuntimeConfiguration = "$(CONFIG_PATH)" @@ -382,6 +385,7 @@ $(GENERATED_FILES): %: %.in Makefile VERSION -e "s|@DEFENABLEDEBUG@|$(DEFENABLEDEBUG)|g" \ -e "s|@DEFDISABLENESTINGCHECKS@|$(DEFDISABLENESTINGCHECKS)|g" \ -e "s|@DEFMSIZE9P@|$(DEFMSIZE9P)|g" \ + -e "s|@DEFHOTPLUGONROOTBUS@|$(DEFHOTPLUGVFIOONROOTBUS)|g" \ $< > $@ generate-config: $(CONFIG) diff --git a/cli/config.go b/cli/config.go index 21d3a17d1e..09f53252c2 100644 --- a/cli/config.go +++ b/cli/config.go @@ -79,12 +79,12 @@ type hypervisor struct { MachineAccelerators string `toml:"machine_accelerators"` KernelParams string `toml:"kernel_params"` MachineType string `toml:"machine_type"` + BlockDeviceDriver string `toml:"block_device_driver"` DefaultVCPUs int32 `toml:"default_vcpus"` DefaultMaxVCPUs uint32 `toml:"default_maxvcpus"` DefaultMemSz uint32 `toml:"default_memory"` DefaultBridges uint32 `toml:"default_bridges"` Msize9p uint32 `toml:"msize_9p"` - BlockDeviceDriver string `toml:"block_device_driver"` DisableBlockDeviceUse bool `toml:"disable_block_device_use"` MemPrealloc bool `toml:"enable_mem_prealloc"` HugePages bool `toml:"enable_hugepages"` @@ -93,6 +93,7 @@ type hypervisor struct { DisableNestingChecks bool `toml:"disable_nesting_checks"` EnableIOThreads bool `toml:"enable_iothreads"` UseVSock bool `toml:"use_vsock"` + HotplugVFIOOnRootBus bool `toml:"hotplug_vfio_on_root_bus"` } type proxy struct { @@ -373,6 +374,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { EnableIOThreads: h.EnableIOThreads, Msize9p: h.msize9p(), UseVSock: useVSock, + HotplugVFIOOnRootBus: h.HotplugVFIOOnRootBus, }, nil } @@ -489,6 +491,7 @@ func loadConfiguration(configPath string, ignoreLogging bool) (resolvedConfigPat BlockDeviceDriver: defaultBlockDeviceDriver, EnableIOThreads: defaultEnableIOThreads, Msize9p: defaultMsize9p, + HotplugVFIOOnRootBus: defaultHotplugVFIOOnRootBus, } err = config.InterNetworkModel.SetModel(defaultInterNetworkingModel) diff --git a/cli/config/configuration.toml.in b/cli/config/configuration.toml.in index 2dcd1222bc..c93bc82597 100644 --- a/cli/config/configuration.toml.in +++ b/cli/config/configuration.toml.in @@ -140,6 +140,13 @@ enable_iothreads = @DEFENABLEIOTHREADS@ # Default false #use_vsock = true +# VFIO devices are hotplugged on a bridge by default. +# Enable hotplugging on root bus. This may be required for devices with +# a large PCI bar, as this is a current limitation with hotplugging on +# a bridge. This value is valid for "pc" machine type. +# Default false +#hotplug_vfio_on_root_bus = true + [factory] # VM templating support. Once enabled, new VMs are created from template # using vm cloning. They will share the same initial kernel, initramfs and diff --git a/cli/config_test.go b/cli/config_test.go index 5a29377d25..e89542ce3b 100644 --- a/cli/config_test.go +++ b/cli/config_test.go @@ -33,7 +33,7 @@ type testRuntimeConfig struct { LogPath string } -func makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath, kernelParams, machineType, shimPath, proxyPath, logPath string, disableBlock bool, blockDeviceDriver string, enableIOThreads bool) string { +func makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath, kernelParams, machineType, shimPath, proxyPath, logPath string, disableBlock bool, blockDeviceDriver string, enableIOThreads bool, hotplugVFIOOnRootBus bool) string { return ` # Runtime configuration file @@ -49,6 +49,7 @@ func makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath default_memory = ` + strconv.FormatUint(uint64(defaultMemSize), 10) + ` disable_block_device_use = ` + strconv.FormatBool(disableBlock) + ` enable_iothreads = ` + strconv.FormatBool(enableIOThreads) + ` + hotplug_vfio_on_root_bus = ` + strconv.FormatBool(hotplugVFIOOnRootBus) + ` msize_9p = ` + strconv.FormatUint(uint64(defaultMsize9p), 10) + ` [proxy.kata] @@ -97,8 +98,9 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf disableBlockDevice := true blockDeviceDriver := "virtio-scsi" enableIOThreads := true + hotplugVFIOOnRootBus := true - runtimeConfigFileData := makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath, kernelParams, machineType, shimPath, proxyPath, logPath, disableBlockDevice, blockDeviceDriver, enableIOThreads) + runtimeConfigFileData := makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath, kernelParams, machineType, shimPath, proxyPath, logPath, disableBlockDevice, blockDeviceDriver, enableIOThreads, hotplugVFIOOnRootBus) configPath := path.Join(dir, "runtime.toml") err = createConfig(configPath, runtimeConfigFileData) @@ -138,6 +140,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf DefaultBridges: defaultBridgesCount, Mlock: !defaultEnableSwap, EnableIOThreads: enableIOThreads, + HotplugVFIOOnRootBus: hotplugVFIOOnRootBus, Msize9p: defaultMsize9p, } @@ -617,6 +620,7 @@ func TestNewQemuHypervisorConfig(t *testing.T) { machineType := "machineType" disableBlock := true enableIOThreads := true + hotplugVFIOOnRootBus := true orgVSockDevicePath := utils.VSockDevicePath orgVHostVSockDevicePath := utils.VHostVSockDevicePath defer func() { @@ -633,6 +637,7 @@ func TestNewQemuHypervisorConfig(t *testing.T) { MachineType: machineType, DisableBlockDeviceUse: disableBlock, EnableIOThreads: enableIOThreads, + HotplugVFIOOnRootBus: hotplugVFIOOnRootBus, UseVSock: true, } @@ -688,6 +693,9 @@ func TestNewQemuHypervisorConfig(t *testing.T) { t.Errorf("Expected value for enable IOThreads %v, got %v", enableIOThreads, config.EnableIOThreads) } + if config.HotplugVFIOOnRootBus != hotplugVFIOOnRootBus { + t.Errorf("Expected value for HotplugVFIOOnRootBus %v, got %v", hotplugVFIOOnRootBus, config.HotplugVFIOOnRootBus) + } } func TestNewQemuHypervisorConfigImageAndInitrd(t *testing.T) { @@ -710,6 +718,7 @@ func TestNewQemuHypervisorConfigImageAndInitrd(t *testing.T) { machineType := "machineType" disableBlock := true enableIOThreads := true + hotplugVFIOOnRootBus := true hypervisor := hypervisor{ Path: hypervisorPath, @@ -719,6 +728,7 @@ func TestNewQemuHypervisorConfigImageAndInitrd(t *testing.T) { MachineType: machineType, DisableBlockDeviceUse: disableBlock, EnableIOThreads: enableIOThreads, + HotplugVFIOOnRootBus: hotplugVFIOOnRootBus, } _, err = newQemuHypervisorConfig(hypervisor) diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index 342eb4a52d..6e33c5267b 100644 --- a/cli/kata-env_test.go +++ b/cli/kata-env_test.go @@ -63,6 +63,7 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC disableBlock := true blockStorageDriver := "virtio-scsi" enableIOThreads := true + hotplugVFIOOnRootBus := true // override defaultProxyPath = proxyPath @@ -108,6 +109,7 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC disableBlock, blockStorageDriver, enableIOThreads, + hotplugVFIOOnRootBus, ) configFile = path.Join(prefixDir, "runtime.toml") diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 991b7dfd3d..e0de00c1a5 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -223,6 +223,10 @@ type HypervisorConfig struct { // UseVSock use a vsock for agent communication UseVSock bool + // HotplugVFIOOnRootBus is used to indicate if devices need to be hotplugged on the + // root bus instead of a bridge. + HotplugVFIOOnRootBus bool + // BootToBeTemplate used to indicate if the VM is created to be a template VM BootToBeTemplate bool diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index e45b31f8ed..f62e60e66c 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -40,9 +40,10 @@ type CPUDevice struct { type QemuState struct { Bridges []Bridge // HotpluggedCPUs is the list of CPUs that were hot-added - HotpluggedVCPUs []CPUDevice - HotpluggedMemory int - UUID string + HotpluggedVCPUs []CPUDevice + HotpluggedMemory int + UUID string + HotplugVFIOOnRootBus bool } // qemu is an Hypervisor interface implementation for the Linux qemu hypervisor. @@ -195,6 +196,8 @@ func (q *qemu) init(id string, hypervisorConfig *HypervisorConfig, vmConfig Reso q.Logger().Debug("Creating UUID") q.state.UUID = uuid.Generate().String() + q.state.HotplugVFIOOnRootBus = q.config.HotplugVFIOOnRootBus + // The path might already exist, but in case of VM templating, // we have to create it since the sandbox has not created it yet. if err = os.MkdirAll(filepath.Join(runStoragePath, id), dirMode); err != nil { @@ -736,6 +739,13 @@ func (q *qemu) hotplugVFIODevice(device *config.VFIODev, op operation) error { devID := device.ID if op == addDevice { + // In case HotplugVFIOOnRootBus is true, devices are hotplugged on the root bus + // for pc machine type instead of bridge. This is useful for devices that require + // a large PCI BAR which is a currently a limitation with PCI bridges. + if q.state.HotplugVFIOOnRootBus { + return q.qmpMonitorCh.qmp.ExecuteVFIODeviceAdd(q.qmpMonitorCh.ctx, devID, device.BDF) + } + addr, bridge, err := q.addDeviceToBridge(devID) if err != nil { return err @@ -745,8 +755,10 @@ func (q *qemu) hotplugVFIODevice(device *config.VFIODev, op operation) error { return err } } else { - if err := q.removeDeviceFromBridge(devID); err != nil { - return err + if !q.state.HotplugVFIOOnRootBus { + if err := q.removeDeviceFromBridge(devID); err != nil { + return err + } } if err := q.qmpMonitorCh.qmp.ExecuteDeviceDel(q.qmpMonitorCh.ctx, devID); err != nil {