qemu-options.hx | 11 +++++++++++ target/arm/kvm.c | 3 +++ target/arm/kvm32.c | 5 +++++ target/arm/kvm64.c | 16 ++++++++++++++++ target/arm/kvm_arm.h | 8 ++++++++ vl.c | 22 ++++++++++++++++++++++ 6 files changed, 65 insertions(+)
From: Manish Jaggi <manish.jaggi@cavium.com> QEMU on arm systems use -machine virt -cpu host option for a VM. Migration thus is limited between machines with same cpu. This is a limitation if migration is desired between cpus which are of same family and have only few diferences like bug fixes which have no effect on VM operation. They just differ in say MIDR values. This patchset introduces a command line option -skipinvariant. Invariant registers will be skipped from being restored from guests context on migrated host. Mailing list discussion on this topic: https://www.mail-archive.com/qemu-devel@nongnu.org/msg560043.html Manish Jaggi (3): arm: API to check if the register is invariant arm: Introduce skipinvariant command line option arm: Skip invariant register restore qemu-options.hx | 11 +++++++++++ target/arm/kvm.c | 3 +++ target/arm/kvm32.c | 5 +++++ target/arm/kvm64.c | 16 ++++++++++++++++ target/arm/kvm_arm.h | 8 ++++++++ vl.c | 22 ++++++++++++++++++++++ 6 files changed, 65 insertions(+) -- 1.8.3.1
On 27 September 2018 at 02:13, <mjaggi@caviumnetworks.com> wrote: > From: Manish Jaggi <manish.jaggi@cavium.com> > > QEMU on arm systems use -machine virt -cpu host option for a VM. > Migration thus is limited between machines with same cpu. > > This is a limitation if migration is desired between cpus which are of same > family and have only few diferences like bug fixes which have no effect on > VM operation. They just differ in say MIDR values. > > This patchset introduces a command line option -skipinvariant. Invariant > registers will be skipped from being restored from guests context on migrated > host. > > Mailing list discussion on this topic: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg560043.html Hi; thanks for this patch. The issue I see with this patch is that the KVM/ARM QEMU approach to system registers so far has been "the kernel knows about these and it is in control". So we ask the kernel for the list of registers, and just save and restore those. That would suggest that if there are sysregs where it's OK in fact to ignore a difference between two constant register values, it should be the kernel doing the "actually, this mismatch is OK" behaviour... For instance, it's probably OK to ignore a MIDR_EL1 difference that just indicates a minor revision bump; but not to ignore one that indicates you just tried to migrate a Cortex-A53 over to a Cavium CPU. thanks -- PMM
On Tue, Oct 02, 2018 at 02:07:38PM +0100, Peter Maydell wrote: > On 27 September 2018 at 02:13, <mjaggi@caviumnetworks.com> wrote: > > From: Manish Jaggi <manish.jaggi@cavium.com> > > > > QEMU on arm systems use -machine virt -cpu host option for a VM. > > Migration thus is limited between machines with same cpu. > > > > This is a limitation if migration is desired between cpus which are of same > > family and have only few diferences like bug fixes which have no effect on > > VM operation. They just differ in say MIDR values. > > > > This patchset introduces a command line option -skipinvariant. Invariant > > registers will be skipped from being restored from guests context on migrated > > host. > > > > Mailing list discussion on this topic: > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg560043.html > > Hi; thanks for this patch. The issue I see with this patch > is that the KVM/ARM QEMU approach to system registers so far > has been "the kernel knows about these and it is in control". > So we ask the kernel for the list of registers, and just save > and restore those. That would suggest that if there are sysregs > where it's OK in fact to ignore a difference between two constant > register values, it should be the kernel doing the "actually, this > mismatch is OK" behaviour... I don't think the kernel should have to maintain all that logic. If a user wants to load guest registers, then the kernel should do it, as long as it's safe from the host's integrity/security PoV, and the hardware would actually support it. Anything that can only break the guest, but not the host, should be allowed. The KVM userspace can certainly ask the kernel what it recommends first (i.e. read the invariant registers first, before deciding what to write), but the decision of what to write should be left up to the user. > > For instance, it's probably OK to ignore a MIDR_EL1 difference > that just indicates a minor revision bump; but not to ignore > one that indicates you just tried to migrate a Cortex-A53 > over to a Cavium CPU. If the user does that, then the guest will break - oh well. That's not the host kernel's problem. Thanks, drew
On 4 October 2018 at 16:05, Andrew Jones <drjones@redhat.com> wrote: > On Tue, Oct 02, 2018 at 02:07:38PM +0100, Peter Maydell wrote: >> Hi; thanks for this patch. The issue I see with this patch >> is that the KVM/ARM QEMU approach to system registers so far >> has been "the kernel knows about these and it is in control". >> So we ask the kernel for the list of registers, and just save >> and restore those. That would suggest that if there are sysregs >> where it's OK in fact to ignore a difference between two constant >> register values, it should be the kernel doing the "actually, this >> mismatch is OK" behaviour... > > I don't think the kernel should have to maintain all that logic. If a user > wants to load guest registers, then the kernel should do it, as long as > it's safe from the host's integrity/security PoV, and the hardware would > actually support it. Anything that can only break the guest, but not the > host, should be allowed. The KVM userspace can certainly ask the kernel > what it recommends first (i.e. read the invariant registers first, before > deciding what to write), but the decision of what to write should be left > up to the user. I can see the logic of that approach. But right now QEMU userspace knows basically nothing about the system registers when we're using KVM: all that knowledge is in the kernel. So we don't have a place really to put policy info (and not really anywhere to put policy info that depends on the host CPU type when mostly we let the kernel care about that). >> For instance, it's probably OK to ignore a MIDR_EL1 difference >> that just indicates a minor revision bump; but not to ignore >> one that indicates you just tried to migrate a Cortex-A53 >> over to a Cavium CPU. > > If the user does that, then the guest will break - oh well. That's not the > host kernel's problem. Patching QEMU to ignore all attempts to write wrong values to read-only registers would be easy. It just wouldn't be very helpful to the user... thanks -- PMM
* Peter Maydell (peter.maydell@linaro.org) wrote: > On 4 October 2018 at 16:05, Andrew Jones <drjones@redhat.com> wrote: > > On Tue, Oct 02, 2018 at 02:07:38PM +0100, Peter Maydell wrote: > >> Hi; thanks for this patch. The issue I see with this patch > >> is that the KVM/ARM QEMU approach to system registers so far > >> has been "the kernel knows about these and it is in control". > >> So we ask the kernel for the list of registers, and just save > >> and restore those. That would suggest that if there are sysregs > >> where it's OK in fact to ignore a difference between two constant > >> register values, it should be the kernel doing the "actually, this > >> mismatch is OK" behaviour... > > > > I don't think the kernel should have to maintain all that logic. If a user > > wants to load guest registers, then the kernel should do it, as long as > > it's safe from the host's integrity/security PoV, and the hardware would > > actually support it. Anything that can only break the guest, but not the > > host, should be allowed. The KVM userspace can certainly ask the kernel > > what it recommends first (i.e. read the invariant registers first, before > > deciding what to write), but the decision of what to write should be left > > up to the user. > > I can see the logic of that approach. But right now QEMU > userspace knows basically nothing about the system registers > when we're using KVM: all that knowledge is in the kernel. > So we don't have a place really to put policy info (and > not really anywhere to put policy info that depends on the > host CPU type when mostly we let the kernel care about that). It would seem wrong to put this logic in the kernel because the priviliged code should be as small as possible. Dave > >> For instance, it's probably OK to ignore a MIDR_EL1 difference > >> that just indicates a minor revision bump; but not to ignore > >> one that indicates you just tried to migrate a Cortex-A53 > >> over to a Cavium CPU. > > > > If the user does that, then the guest will break - oh well. That's not the > > host kernel's problem. > > Patching QEMU to ignore all attempts to write wrong values > to read-only registers would be easy. It just wouldn't > be very helpful to the user... > > thanks > -- PMM -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 5 October 2018 at 09:48, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote: > * Peter Maydell (peter.maydell@linaro.org) wrote: >> On 4 October 2018 at 16:05, Andrew Jones <drjones@redhat.com> wrote: >> > On Tue, Oct 02, 2018 at 02:07:38PM +0100, Peter Maydell wrote: >> >> Hi; thanks for this patch. The issue I see with this patch >> >> is that the KVM/ARM QEMU approach to system registers so far >> >> has been "the kernel knows about these and it is in control". >> >> So we ask the kernel for the list of registers, and just save >> >> and restore those. That would suggest that if there are sysregs >> >> where it's OK in fact to ignore a difference between two constant >> >> register values, it should be the kernel doing the "actually, this >> >> mismatch is OK" behaviour... >> > >> > I don't think the kernel should have to maintain all that logic. If a user >> > wants to load guest registers, then the kernel should do it, as long as >> > it's safe from the host's integrity/security PoV, and the hardware would >> > actually support it. Anything that can only break the guest, but not the >> > host, should be allowed. The KVM userspace can certainly ask the kernel >> > what it recommends first (i.e. read the invariant registers first, before >> > deciding what to write), but the decision of what to write should be left >> > up to the user. >> >> I can see the logic of that approach. But right now QEMU >> userspace knows basically nothing about the system registers >> when we're using KVM: all that knowledge is in the kernel. >> So we don't have a place really to put policy info (and >> not really anywhere to put policy info that depends on the >> host CPU type when mostly we let the kernel care about that). > > It would seem wrong to put this logic in the kernel because the > priviliged code should be as small as possible. Agreed. But if we want to move to "QEMU knows about all the possible host CPUs" that's a fair-sized design change and needs more than the minimalist approach this patch has. thanks -- PMM
From: Manish Jaggi <manish.jaggi@cavium.com>
Invariant registers will be skipped from being restored from
guests' context on migrated host.
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 65f867d..2d89600 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -451,6 +451,9 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
default:
abort();
}
+ if (skip_invariant && kvm_arm_is_invariant(&r)) {
+ continue;
+ }
ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
if (ret) {
/* We might fail for "unknown register" and also for
--
1.8.3.1
On Thu, Sep 27, 2018 at 01:13:54AM +0000, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <manish.jaggi@cavium.com>
>
> Invariant registers will be skipped from being restored from
> guests' context on migrated host.
>
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 65f867d..2d89600 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -451,6 +451,9 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
> default:
> abort();
> }
> + if (skip_invariant && kvm_arm_is_invariant(&r)) {
> + continue;
> + }
> ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
> if (ret) {
> /* We might fail for "unknown register" and also for
> --
> 1.8.3.1
>
>
I think we should compare the invariants we're going to skip restoring
with their saved state and output messages when they don't match to the
migration log. That way when things go wrong we have a clue as to why.
Thanks,
drew
© 2016 - 2024 Red Hat, Inc.