In near future it will be necessary to know the PID of DBus daemon started for QEMU. Move the code into a separate function (qemuDBusGetPID()) and export it in the header file. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/qemu/qemu_dbus.c | 42 +++++++++++++++++++++++++++++------------- src/qemu/qemu_dbus.h | 4 ++++ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c index cb2694795e..775baecc8e 100644 --- a/src/qemu/qemu_dbus.c +++ b/src/qemu/qemu_dbus.c @@ -143,28 +143,44 @@ qemuDBusStop(virQEMUDriver *driver, } +int +qemuDBusGetPID(virQEMUDriver *driver, + virDomainObj *vm, + pid_t *pid) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivate *priv = vm->privateData; + g_autofree char *shortName = NULL; + g_autofree char *pidfile = NULL; + + if (!priv->dbusDaemonRunning) + return 0; + + if (!(shortName = virDomainDefGetShortName(vm->def))) + return -1; + pidfile = qemuDBusCreatePidFilename(cfg, shortName); + if (virPidFileReadPath(pidfile, pid) < 0) { + VIR_WARN("Unable to get DBus PID"); + return -1; + } + + return 0; +} + + int qemuDBusSetupCgroup(virQEMUDriver *driver, virDomainObj *vm, virCgroup *cgroup) { - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - qemuDomainObjPrivate *priv = vm->privateData; - g_autofree char *shortName = NULL; - g_autofree char *pidfile = NULL; pid_t cpid = -1; - if (!priv->dbusDaemonRunning) + if (qemuDBusGetPID(driver, vm, &cpid) < 0) + return -1; + + if (cpid == -1) return 0; - if (!(shortName = virDomainDefGetShortName(vm->def))) - return -1; - pidfile = qemuDBusCreatePidFilename(cfg, shortName); - if (virPidFileReadPath(pidfile, &cpid) < 0) { - VIR_WARN("Unable to get DBus PID"); - return -1; - } - return virCgroupAddProcess(cgroup, cpid); } diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h index b27f38a591..a079976aa4 100644 --- a/src/qemu/qemu_dbus.h +++ b/src/qemu/qemu_dbus.h @@ -34,6 +34,10 @@ void qemuDBusVMStateAdd(virDomainObj *vm, const char *id); void qemuDBusVMStateRemove(virDomainObj *vm, const char *id); +int qemuDBusGetPID(virQEMUDriver *driver, + virDomainObj *vm, + pid_t *pid); + int qemuDBusSetupCgroup(virQEMUDriver *driver, virDomainObj *vm, virCgroup *cgroup); -- 2.35.1

In near future it will be necessary to know the PID of swtpm process for QEMU. Export the function that does just that (qemuTPMEmulatorGetPid()). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/qemu/qemu_tpm.c | 2 +- src/qemu/qemu_tpm.h | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 03829775b8..49237c6be5 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -138,7 +138,7 @@ qemuTPMEmulatorPidFileBuildPath(const char *swtpmStateDir, * If the PID was not still alive, zero will be returned, and @pid will be * set to -1; */ -static int +int qemuTPMEmulatorGetPid(const char *swtpmStateDir, const char *shortName, pid_t *pid) diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h index 9951f025a6..9f4d01f60b 100644 --- a/src/qemu/qemu_tpm.h +++ b/src/qemu/qemu_tpm.h @@ -50,6 +50,13 @@ void qemuExtTPMStop(virQEMUDriver *driver, virDomainObj *vm) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuTPMEmulatorGetPid(const char *swtpmStateDir, + const char *shortName, + pid_t *pid) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_NONNULL(3) + G_GNUC_WARN_UNUSED_RESULT; + int qemuExtTPMSetupCgroup(virQEMUDriver *driver, virDomainDef *def, virCgroup *cgroup) -- 2.35.1

Since its 5.14 release the Linux kernel allows userspace to define trusted groups of processes/threads that can run on sibling Hyper Threads (HT) at the same time. This is to mitigate side channel attacks like L1TF or MDS. If there are no tasks to fully utilize all HTs, then a HT will idle instead of running a task from another (un-)trusted group. On low level, this is implemented by cookies (effectively an UL value): processes in the same trusted group share the same cookie and cookie is unique to the group. There are four basic operations: 1) PR_SCHED_CORE_GET -- get cookie of given PID, 2) PR_SCHED_CORE_CREATE -- create a new unique cookie for PID, 3) PR_SCHED_CORE_SHARE_TO -- push cookie of the caller onto another PID, 4) PR_SCHED_CORE_SHARE_FROM -- pull cookie of another PID into the caller. Since a system where the code is built can be different to the one where the code is ran let's provide declaration of some values. It's not unusual for distros to ship older linux-headers than the actual kernel. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/libvirt_private.syms | 4 ++ src/util/virprocess.c | 124 +++++++++++++++++++++++++++++++++++++++ src/util/virprocess.h | 8 +++ 3 files changed, 136 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 76bcc64eb0..443a63444b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3137,6 +3137,10 @@ virProcessKillPainfullyDelay; virProcessNamespaceAvailable; virProcessRunInFork; virProcessRunInMountNamespace; +virProcessSchedCoreAvailable; +virProcessSchedCoreCreate; +virProcessSchedCoreShareFrom; +virProcessSchedCoreShareTo; virProcessSchedPolicyTypeFromString; virProcessSchedPolicyTypeToString; virProcessSetAffinity; diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 013afd91b4..a59e64970d 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -56,6 +56,10 @@ # include <windows.h> #endif +#ifdef __linux__ +# include <sys/prctl.h> +#endif + #include "virprocess.h" #include "virerror.h" #include "viralloc.h" @@ -1874,3 +1878,123 @@ virProcessGetSchedInfo(unsigned long long *cpuWait, return 0; } #endif /* __linux__ */ + +#ifdef __linux__ +# ifndef PR_SCHED_CORE +/* Copied from linux/prctl.h */ +# define PR_SCHED_CORE 62 +# define PR_SCHED_CORE_GET 0 +# define PR_SCHED_CORE_CREATE 1 /* create unique core_sched cookie */ +# define PR_SCHED_CORE_SHARE_TO 2 /* push core_sched cookie to pid */ +# define PR_SCHED_CORE_SHARE_FROM 3 /* pull core_sched cookie to pid */ +# endif + +/* Unfortunately, kernel-headers forgot to export these. */ +# ifndef PR_SCHED_CORE_SCOPE_THREAD +# define PR_SCHED_CORE_SCOPE_THREAD 0 +# define PR_SCHED_CORE_SCOPE_THREAD_GROUP 1 +# define PR_SCHED_CORE_SCOPE_PROCESS_GROUP 2 +# endif + +/** + * virProcessSchedCoreAvailable: + * + * Check whether kernel supports Core Scheduling (CONFIG_SCHED_CORE), i.e. only + * a defined set of PIDs/TIDs can run on sibling Hyper Threads at the same + * time. + * + * Returns: 1 if Core Scheduling is available, + * 0 if Core Scheduling is NOT available, + * -1 otherwise. + */ +int +virProcessSchedCoreAvailable(void) +{ + unsigned long cookie = 0; + int rc; + + /* Let's just see if we can get our own sched cookie, and if yes we can + * safely assume CONFIG_SCHED_CORE kernel is available. */ + rc = prctl(PR_SCHED_CORE, PR_SCHED_CORE_GET, 0, + PR_SCHED_CORE_SCOPE_THREAD, &cookie); + + return rc == 0 ? 1 : errno == EINVAL ? 0 : -1; +} + +/** + * virProcessSchedCoreCreate: + * + * Creates a new trusted group for the caller process. + * + * Returns: 0 on success, + * -1 otherwise, with errno set. + */ +int +virProcessSchedCoreCreate(void) +{ + /* pid = 0 (3rd argument) means the calling process. */ + return prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, 0, + PR_SCHED_CORE_SCOPE_THREAD_GROUP, 0); +} + +/** + * virProcessSchedCoreShareFrom: + * @pid: PID to share group with + * + * Places the current caller process into the trusted group of @pid. + * + * Returns: 0 on success, + * -1 otherwise, with errno set. + */ +int +virProcessSchedCoreShareFrom(pid_t pid) +{ + return prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_FROM, pid, + PR_SCHED_CORE_SCOPE_THREAD, 0); +} + +/** + * virProcessSchedCoreShareTo: + * @pid: PID to share group with + * + * Places foreign @pid into the trusted group of the current caller process. + * + * Returns: 0 on success, + * -1 otherwise, with errno set. + */ +int +virProcessSchedCoreShareTo(pid_t pid) +{ + return prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, pid, + PR_SCHED_CORE_SCOPE_THREAD, 0); +} + +#else /* !__linux__ */ + +int +virProcessSchedCoreAvailable(void) +{ + return 0; +} + +int +virProcessSchedCoreCreate(void) +{ + errno = ENOSYS; + return -1; +} + +int +virProcessSchedCoreShareFrom(pid_t pid G_GNUC_UNUSED) +{ + errno = ENOSYS; + return -1; +} + +int +virProcessSchedCoreShareTo(pid_t pid G_GNUC_UNUSED) +{ + errno = ENOSYS; + return -1; +} +#endif /* !__linux__ */ diff --git a/src/util/virprocess.h b/src/util/virprocess.h index 086fbe0e4d..e01f9a24ee 100644 --- a/src/util/virprocess.h +++ b/src/util/virprocess.h @@ -202,3 +202,11 @@ int virProcessGetStatInfo(unsigned long long *cpuTime, int virProcessGetSchedInfo(unsigned long long *cpuWait, pid_t pid, pid_t tid); + +int virProcessSchedCoreAvailable(void); + +int virProcessSchedCoreCreate(void); + +int virProcessSchedCoreShareFrom(pid_t pid); + +int virProcessSchedCoreShareTo(pid_t pid); -- 2.35.1

On Mon, Jun 27, 2022 at 12:44:39PM +0200, Michal Privoznik wrote: Talking to the OpenStack Nova maintainers I'm remembering that life is somewhat more complicated than we have taken into account. Nova has a variety of tunables along semi-independant axes which can be combined * CPU policy: shared vs dedicated - this is the big one, with overcommit being the out of the box default. In both cases they apply CPU pinning to the QEMU VM. In the case of shared, they pin to allow the VM to float freely over all host CPUs, except for a small subset reserved for the host OS. This explicitly overcommits host resources. In the case of dedicated, they pin to give each vCPU a corresponduing unique vCPU. There is broadly no overcommit of vCPUs. non-vCPU threads may still overcommit/compete * SMT policy: prefer vs isolate vs required For 'prefer', it'll preferentially pick a host with SMT and give the VM SMT siblings, but will fallback to non-SMT hosts if not possible For 'isolate', it'll keep all-but-1 SMT sibling empty of VCPUs at all times For 'require', it'll mandate a host with SMT and give the VM SMT siblings * Emulator policy: float vs isolate vs shared For 'follow', the emulator threads will float across pCPUs assign to the same guest's vCPUs For 'isolate', the emulator threads will be pinned to pCPU(s) separate from the vCPUs. These pCPU can be chosen in two different ways though - Each VM is strictly given its own pCPU just for its own emulator threads. Typically used with RealTime - Each VM is given pCPU(s) for its emulator thread that can be shared with other VMs. Typically used with non-RealTime In terms of core scheduling usage - For the default shared model, where all VM CPUs float and overcommit, if we enable core scheduling the capacity of a host decreases. Biggest impact is if there are many guests with odd-CPU counts, OR many guests with even CPU counts but with only 1 runnable CPU at a time. - When emulator threads policy is 'isolate', our core scheduling setup could massively conflict with Nova's emulator placement. eg Nova could have given SMT siblings to two different guests for their respective emumalator threads. This is not as serious a security risk as sharing SMT siblings with VCPUs, as emulator thread code is trustworthy, unless QEMU was exploited The net result is that even if 2 VMs have their vCPUs runnable and host has pCPUs available to run them, one VM can be stalled by its emulator thread pinning having an SMT core scheduling conflict with the other VM's emulator thread pinning. Nova can also mix-and-match the above policies between VMs in the same host. Finally, the above is all largely focused on VM placement. One thing to bear in mind is that even if VMs are isolated from SMT siblings, Nova deployments can still allow host OS processes to co-exist with vCPU threads on SMT siblings. Our core scheduling impl would prevent that The goal for core scheduling is to make the free-floating overcommit scenario as safe as dedicated CPU pinning, while retaining the flexibility of dynamic placement. Core scheduling is redundant if the mgmt app has given dedicate CPU pinning to all vCPUS and all other threads. At the libvirt side though, we don't know whether Nova is doing overcommit or dedicated CPUs. CPU pinning masks will be given in both cases, the only difference is that in the dedicated case, the mask is highly likely only list 1 pCPU bit. I don't think we want to try to infer intent by looking at CPU masks though. What this means is that if we apply core scheduling by default, it needs to be compatible with the combination of all the above options. On reflection, I don't think we can achieve that with high enough confidence. There's a decent case to be made that libvirt's core schedule would be good for the overcommit case out of the box, even though it has a capacity impact, but the ability of host OS threads to co-exist with vCPU threads would be broken. It is clear that applying core scheduling on top of nova's dedicated CPU pinnning policies can be massively harmful wrt to emulator threads policies too. So what I'm thinking is that our set of three options here are not sufficient, we need more: "none" - nor QEMU nor any of its helper processes are placed into separate scheduling group "vcpus" - only QEMU vCPU threads are placed into a separate scheduling group, emulator threads and helper processes remain outside of the group "emulator" - only QEMU and its threads (emulator + vCPUs) are placed into separate scheduling group, helper proccesses remain outside of the group. "full" - both QEMU and its helper processes are placed into separate scheduling group. I don't think any of the three core scheduling options is safe enough to use by default though. They all have a decent chance of causing regressions for Nova, even though they'll improve security. So reluctantly I think we need to default to "none" and require opt in. Given Nova's mix/match of VM placement settings, I also think that a per-VM XML knob is more likely to be neccessary than we originally believed. At the very least being able to switch between 'vcpus' and 'emulator' modes feels reasonably important. Of course Nova is just one mgmt app, but we can assume that there exists other apps that will credibly have the same requirements and thus risks of regressions. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Mon, Jun 27, 2022 at 12:44:40PM +0200, Michal Privoznik wrote: The next patch deals with helper processes too. I guess the difference in this patch is that it deals with helper processes spawned /after/ QEMU, so they can inherit scheduling group at startup easily, while the next patch has to apply the group later in startup ? With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Since the level of trust that QEMU has is the same level of trust that helper processes have there's no harm in placing all of them into the same group. Unfortunately, since these processes are started before QEMU we can't use brand new virCommand*() APIs (those are used on hotplug though) and have to use the low level virProcess*() APIs. Moreover, because there no (kernel) API that would copy cookie from one process to another WITHOUT modifying the cookie of the process that's doing the copy, we have to fork() and use available copy APIs. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_extdevice.c | 120 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_extdevice.h | 3 + src/qemu/qemu_process.c | 4 ++ 3 files changed, 127 insertions(+) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index b8e3c1000a..41368a9cea 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -335,3 +335,123 @@ qemuExtDevicesSetupCgroup(virQEMUDriver *driver, return 0; } + + +static int +qemuExtDevicesSetupSchedHelper(pid_t ppid G_GNUC_UNUSED, + void *opaque) +{ + GSList *pids = opaque; + GSList *next; + pid_t vmPid; + + /* The first item on the list is special: it's the PID of the + * QEMU that has the cookie we want to copy to the rest. */ + vmPid = GPOINTER_TO_INT(pids->data); + if (virProcessSchedCoreShareFrom(vmPid) < 0) { + virReportSystemError(errno, + _("Unable to get core group of: %lld"), + (long long) vmPid); + return -1; + } + + VIR_DEBUG("SCHED_CORE: vmPid = %lld", (long long) vmPid); + + for (next = pids->next; next; next = next->next) { + pid_t pid = GPOINTER_TO_INT(next->data); + + VIR_DEBUG("SCHED_CORE: share to %lld", (long long) pid); + if (virProcessSchedCoreShareTo(pid) < 0) { + virReportSystemError(errno, + _("Unable to share core group to: %lld"), + (long long) pid); + return -1; + } + } + + return 0; +} + + +int +qemuExtDevicesSetupSched(virQEMUDriver *driver, + virDomainObj *vm) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + virDomainDef *def = vm->def; + g_autofree char *shortname = NULL; + g_autoptr(GSList) pids = NULL; + size_t i; + pid_t cpid = -1; + + if (cfg->schedCore != QEMU_SCHED_CORE_FULL) + return 0; + + shortname = virDomainDefGetShortName(def); + if (!shortname) + return -1; + + if (qemuDBusGetPID(driver, vm, &cpid) < 0) + return -1; + + if (cpid != -1) + pids = g_slist_prepend(pids, GINT_TO_POINTER(cpid)); + + for (i = 0; i < def->nvideos; i++) { + virDomainVideoDef *video = def->videos[i]; + + if (video->backend != VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) + continue; + + if (qemuVhostUserGPUGetPid(cfg->stateDir, shortname, video->info.alias, &cpid) < 0) + return -1; + + if (cpid != -1) + pids = g_slist_prepend(pids, GINT_TO_POINTER(cpid)); + } + + for (i = 0; i < def->nnets; i++) { + virDomainNetDef *net = def->nets[i]; + qemuSlirp *slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp; + + if (slirp && slirp->pid != -1) + pids = g_slist_prepend(pids, GINT_TO_POINTER(slirp->pid)); + } + + for (i = 0; i < def->ntpms; i++) { + virDomainTPMDef *tpm = def->tpms[i]; + + if (tpm->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) + continue; + + if (qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortname, &cpid) < 0) + return -1; + + if (cpid != -1) + pids = g_slist_prepend(pids, GINT_TO_POINTER(cpid)); + } + + for (i = 0; i < def->nfss; i++) { + virDomainFSDef *fs = def->fss[i]; + + if (fs->sock || + fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) + continue; + + if (qemuVirtioFSGetPid(vm, fs, &cpid) < 0) + return -1; + + if (cpid != -1) + pids = g_slist_prepend(pids, GINT_TO_POINTER(cpid)); + } + + /* Exit early if there's nothing to do, to avoid needless fork. */ + if (!pids) + return 0; + + pids = g_slist_prepend(pids, GINT_TO_POINTER(vm->pid)); + + /* Unfortunately, there's no better way of copying scheduling + * cookies than fork(). */ + return virProcessRunInFork(qemuExtDevicesSetupSchedHelper, pids); +} diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 43d2a4dfff..02397adc6c 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -59,3 +59,6 @@ bool qemuExtDevicesHasDevice(virDomainDef *def); int qemuExtDevicesSetupCgroup(virQEMUDriver *driver, virDomainObj *vm, virCgroup *cgroup); + +int qemuExtDevicesSetupSched(virQEMUDriver *driver, + virDomainObj *vm); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 86c058316f..eb8dfb8f11 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7714,6 +7714,10 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuSetupCgroupForExtDevices(vm, driver) < 0) goto cleanup; + VIR_DEBUG("Setting SCHED_CORE for external devices (if required)"); + if (qemuExtDevicesSetupSched(driver, vm) < 0) + goto cleanup; + VIR_DEBUG("Setting up resctrl"); if (qemuProcessResctrlCreate(driver, vm) < 0) goto cleanup; -- 2.35.1