* [PATCH 1/2] KVM: PPC: Book3S HV: XIVE: initialize private pointer when VPs are allocated
@ 2019-09-10 13:43 ` Greg Kurz
0 siblings, 0 replies; 6+ messages in thread
From: Greg Kurz @ 2019-09-10 13:43 UTC (permalink / raw)
To: Paul Mackerras; +Cc: David Gibson, Cédric Le Goater, kvm-ppc, kvm
From: Cédric Le Goater <clg@kaod.org>
Do not assign the device private pointer before making sure the XIVE
VPs are allocated in OPAL and test pointer validity when releasing
the device.
Fixes: 5422e95103cf ("KVM: PPC: Book3S HV: XIVE: Replace the 'destroy' method by a 'release' method")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
arch/powerpc/kvm/book3s_xive.c | 13 +++++++++++--
arch/powerpc/kvm/book3s_xive_native.c | 13 +++++++++++--
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 591bfb4bfd0f..cd2006bfcd3e 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -1897,12 +1897,21 @@ void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
static void kvmppc_xive_release(struct kvm_device *dev)
{
struct kvmppc_xive *xive = dev->private;
- struct kvm *kvm = xive->kvm;
+ struct kvm *kvm;
struct kvm_vcpu *vcpu;
int i;
pr_devel("Releasing xive device\n");
+ /*
+ * In case of failure (OPAL) at device creation, the device
+ * can be partially initialized.
+ */
+ if (!xive)
+ return;
+
+ kvm = xive->kvm;
+
/*
* Since this is the device release function, we know that
* userspace does not have any open fd referring to the
@@ -2001,7 +2010,6 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
if (!xive)
return -ENOMEM;
- dev->private = xive;
xive->dev = dev;
xive->kvm = kvm;
mutex_init(&xive->lock);
@@ -2031,6 +2039,7 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
if (ret)
return ret;
+ dev->private = xive;
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 248c1ea9e788..e9cbb42de424 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -987,12 +987,21 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
static void kvmppc_xive_native_release(struct kvm_device *dev)
{
struct kvmppc_xive *xive = dev->private;
- struct kvm *kvm = xive->kvm;
+ struct kvm *kvm;
struct kvm_vcpu *vcpu;
int i;
pr_devel("Releasing xive native device\n");
+ /*
+ * In case of failure (OPAL) at device creation, the device
+ * can be partially initialized.
+ */
+ if (!xive)
+ return;
+
+ kvm = xive->kvm;
+
/*
* Clear the KVM device file address_space which is used to
* unmap the ESB pages when a device is passed-through.
@@ -1076,7 +1085,6 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
if (!xive)
return -ENOMEM;
- dev->private = xive;
xive->dev = dev;
xive->kvm = kvm;
kvm->arch.xive = xive;
@@ -1100,6 +1108,7 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
if (ret)
return ret;
+ dev->private = xive;
return 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/2] KVM: PPC: Book3S HV: XIVE: initialize private pointer when VPs are allocated
@ 2019-09-10 13:43 ` Greg Kurz
0 siblings, 0 replies; 6+ messages in thread
From: Greg Kurz @ 2019-09-10 13:43 UTC (permalink / raw)
To: Paul Mackerras; +Cc: David Gibson, Cédric Le Goater, kvm-ppc, kvm
From: Cédric Le Goater <clg@kaod.org>
Do not assign the device private pointer before making sure the XIVE
VPs are allocated in OPAL and test pointer validity when releasing
the device.
Fixes: 5422e95103cf ("KVM: PPC: Book3S HV: XIVE: Replace the 'destroy' method by a 'release' method")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
arch/powerpc/kvm/book3s_xive.c | 13 +++++++++++--
arch/powerpc/kvm/book3s_xive_native.c | 13 +++++++++++--
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 591bfb4bfd0f..cd2006bfcd3e 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -1897,12 +1897,21 @@ void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
static void kvmppc_xive_release(struct kvm_device *dev)
{
struct kvmppc_xive *xive = dev->private;
- struct kvm *kvm = xive->kvm;
+ struct kvm *kvm;
struct kvm_vcpu *vcpu;
int i;
pr_devel("Releasing xive device\n");
+ /*
+ * In case of failure (OPAL) at device creation, the device
+ * can be partially initialized.
+ */
+ if (!xive)
+ return;
+
+ kvm = xive->kvm;
+
/*
* Since this is the device release function, we know that
* userspace does not have any open fd referring to the
@@ -2001,7 +2010,6 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
if (!xive)
return -ENOMEM;
- dev->private = xive;
xive->dev = dev;
xive->kvm = kvm;
mutex_init(&xive->lock);
@@ -2031,6 +2039,7 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
if (ret)
return ret;
+ dev->private = xive;
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 248c1ea9e788..e9cbb42de424 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -987,12 +987,21 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
static void kvmppc_xive_native_release(struct kvm_device *dev)
{
struct kvmppc_xive *xive = dev->private;
- struct kvm *kvm = xive->kvm;
+ struct kvm *kvm;
struct kvm_vcpu *vcpu;
int i;
pr_devel("Releasing xive native device\n");
+ /*
+ * In case of failure (OPAL) at device creation, the device
+ * can be partially initialized.
+ */
+ if (!xive)
+ return;
+
+ kvm = xive->kvm;
+
/*
* Clear the KVM device file address_space which is used to
* unmap the ESB pages when a device is passed-through.
@@ -1076,7 +1085,6 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
if (!xive)
return -ENOMEM;
- dev->private = xive;
xive->dev = dev;
xive->kvm = kvm;
kvm->arch.xive = xive;
@@ -1100,6 +1108,7 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
if (ret)
return ret;
+ dev->private = xive;
return 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] KVM: PPC: Book3S HV: XIVE: Set kvm->arch.xive when VPs are allocated
2019-09-10 13:43 ` Greg Kurz
@ 2019-09-10 13:44 ` Greg Kurz
-1 siblings, 0 replies; 6+ messages in thread
From: Greg Kurz @ 2019-09-10 13:44 UTC (permalink / raw)
To: Paul Mackerras; +Cc: David Gibson, Cédric Le Goater, kvm-ppc, kvm
If we cannot allocate the XIVE VPs in OPAL, the creation of a XIVE or
XICS-on-XIVE device is aborted as expected, but we leave kvm->arch.xive
set forever since the relase method isn't called in this case. Any
subsequent tentative to create a XIVE or XICS-on-XIVE for this VM will
thus always fail. This is a problem for QEMU since it destroys and
re-creates these devices when the VM is reset: the VM would be
restricted to using the emulated XIVE or XICS forever.
As an alternative to adding rollback, do not assign kvm->arch.xive before
making sure the XIVE VPs are allocated in OPAL.
Fixes: 5422e95103cf ("KVM: PPC: Book3S HV: XIVE: Replace the 'destroy' method by a 'release' method")
Signed-off-by: Greg Kurz <groug@kaod.org>
---
arch/powerpc/kvm/book3s_xive.c | 11 +++++------
arch/powerpc/kvm/book3s_xive_native.c | 2 +-
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index cd2006bfcd3e..2ef43d037a4f 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -2006,6 +2006,10 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
pr_devel("Creating xive for partition\n");
+ /* Already there ? */
+ if (kvm->arch.xive)
+ return -EEXIST;
+
xive = kvmppc_xive_get_device(kvm, type);
if (!xive)
return -ENOMEM;
@@ -2014,12 +2018,6 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
xive->kvm = kvm;
mutex_init(&xive->lock);
- /* Already there ? */
- if (kvm->arch.xive)
- ret = -EEXIST;
- else
- kvm->arch.xive = xive;
-
/* We use the default queue size set by the host */
xive->q_order = xive_native_default_eq_shift();
if (xive->q_order < PAGE_SHIFT)
@@ -2040,6 +2038,7 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
return ret;
dev->private = xive;
+ kvm->arch.xive = xive;
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index e9cbb42de424..84a354b90f60 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -1087,7 +1087,6 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
xive->dev = dev;
xive->kvm = kvm;
- kvm->arch.xive = xive;
mutex_init(&xive->mapping_lock);
mutex_init(&xive->lock);
@@ -1109,6 +1108,7 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
return ret;
dev->private = xive;
+ kvm->arch.xive = xive;
return 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] KVM: PPC: Book3S HV: XIVE: Set kvm->arch.xive when VPs are allocated
@ 2019-09-10 13:44 ` Greg Kurz
0 siblings, 0 replies; 6+ messages in thread
From: Greg Kurz @ 2019-09-10 13:44 UTC (permalink / raw)
To: Paul Mackerras; +Cc: David Gibson, Cédric Le Goater, kvm-ppc, kvm
If we cannot allocate the XIVE VPs in OPAL, the creation of a XIVE or
XICS-on-XIVE device is aborted as expected, but we leave kvm->arch.xive
set forever since the relase method isn't called in this case. Any
subsequent tentative to create a XIVE or XICS-on-XIVE for this VM will
thus always fail. This is a problem for QEMU since it destroys and
re-creates these devices when the VM is reset: the VM would be
restricted to using the emulated XIVE or XICS forever.
As an alternative to adding rollback, do not assign kvm->arch.xive before
making sure the XIVE VPs are allocated in OPAL.
Fixes: 5422e95103cf ("KVM: PPC: Book3S HV: XIVE: Replace the 'destroy' method by a 'release' method")
Signed-off-by: Greg Kurz <groug@kaod.org>
---
arch/powerpc/kvm/book3s_xive.c | 11 +++++------
arch/powerpc/kvm/book3s_xive_native.c | 2 +-
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index cd2006bfcd3e..2ef43d037a4f 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -2006,6 +2006,10 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
pr_devel("Creating xive for partition\n");
+ /* Already there ? */
+ if (kvm->arch.xive)
+ return -EEXIST;
+
xive = kvmppc_xive_get_device(kvm, type);
if (!xive)
return -ENOMEM;
@@ -2014,12 +2018,6 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
xive->kvm = kvm;
mutex_init(&xive->lock);
- /* Already there ? */
- if (kvm->arch.xive)
- ret = -EEXIST;
- else
- kvm->arch.xive = xive;
-
/* We use the default queue size set by the host */
xive->q_order = xive_native_default_eq_shift();
if (xive->q_order < PAGE_SHIFT)
@@ -2040,6 +2038,7 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
return ret;
dev->private = xive;
+ kvm->arch.xive = xive;
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index e9cbb42de424..84a354b90f60 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -1087,7 +1087,6 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
xive->dev = dev;
xive->kvm = kvm;
- kvm->arch.xive = xive;
mutex_init(&xive->mapping_lock);
mutex_init(&xive->lock);
@@ -1109,6 +1108,7 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
return ret;
dev->private = xive;
+ kvm->arch.xive = xive;
return 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] KVM: PPC: Book3S HV: XIVE: Set kvm->arch.xive when VPs are allocated
2019-09-10 13:44 ` Greg Kurz
@ 2019-09-10 13:49 ` Cédric Le Goater
-1 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2019-09-10 13:49 UTC (permalink / raw)
To: Greg Kurz, Paul Mackerras; +Cc: David Gibson, kvm-ppc, kvm
On 10/09/2019 15:44, Greg Kurz wrote:
> If we cannot allocate the XIVE VPs in OPAL, the creation of a XIVE or
> XICS-on-XIVE device is aborted as expected, but we leave kvm->arch.xive
> set forever since the relase method isn't called in this case. Any
> subsequent tentative to create a XIVE or XICS-on-XIVE for this VM will
> thus always fail. This is a problem for QEMU since it destroys and
> re-creates these devices when the VM is reset: the VM would be
> restricted to using the emulated XIVE or XICS forever.
>
> As an alternative to adding rollback, do not assign kvm->arch.xive before
> making sure the XIVE VPs are allocated in OPAL.
>
> Fixes: 5422e95103cf ("KVM: PPC: Book3S HV: XIVE: Replace the 'destroy' method by a 'release' method")
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> arch/powerpc/kvm/book3s_xive.c | 11 +++++------
> arch/powerpc/kvm/book3s_xive_native.c | 2 +-
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index cd2006bfcd3e..2ef43d037a4f 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -2006,6 +2006,10 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
>
> pr_devel("Creating xive for partition\n");
>
> + /* Already there ? */
> + if (kvm->arch.xive)
> + return -EEXIST;
> +
> xive = kvmppc_xive_get_device(kvm, type);
> if (!xive)
> return -ENOMEM;
> @@ -2014,12 +2018,6 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
> xive->kvm = kvm;
> mutex_init(&xive->lock);
>
> - /* Already there ? */
> - if (kvm->arch.xive)
> - ret = -EEXIST;
> - else
> - kvm->arch.xive = xive;
> -
> /* We use the default queue size set by the host */
> xive->q_order = xive_native_default_eq_shift();
> if (xive->q_order < PAGE_SHIFT)
> @@ -2040,6 +2038,7 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
> return ret;
>
> dev->private = xive;
> + kvm->arch.xive = xive;
> return 0;
> }
>
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index e9cbb42de424..84a354b90f60 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -1087,7 +1087,6 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
>
> xive->dev = dev;
> xive->kvm = kvm;
> - kvm->arch.xive = xive;
> mutex_init(&xive->mapping_lock);
> mutex_init(&xive->lock);
>
> @@ -1109,6 +1108,7 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
> return ret;
>
> dev->private = xive;
> + kvm->arch.xive = xive;
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] KVM: PPC: Book3S HV: XIVE: Set kvm->arch.xive when VPs are allocated
@ 2019-09-10 13:49 ` Cédric Le Goater
0 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2019-09-10 13:49 UTC (permalink / raw)
To: Greg Kurz, Paul Mackerras; +Cc: David Gibson, kvm-ppc, kvm
On 10/09/2019 15:44, Greg Kurz wrote:
> If we cannot allocate the XIVE VPs in OPAL, the creation of a XIVE or
> XICS-on-XIVE device is aborted as expected, but we leave kvm->arch.xive
> set forever since the relase method isn't called in this case. Any
> subsequent tentative to create a XIVE or XICS-on-XIVE for this VM will
> thus always fail. This is a problem for QEMU since it destroys and
> re-creates these devices when the VM is reset: the VM would be
> restricted to using the emulated XIVE or XICS forever.
>
> As an alternative to adding rollback, do not assign kvm->arch.xive before
> making sure the XIVE VPs are allocated in OPAL.
>
> Fixes: 5422e95103cf ("KVM: PPC: Book3S HV: XIVE: Replace the 'destroy' method by a 'release' method")
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> arch/powerpc/kvm/book3s_xive.c | 11 +++++------
> arch/powerpc/kvm/book3s_xive_native.c | 2 +-
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index cd2006bfcd3e..2ef43d037a4f 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -2006,6 +2006,10 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
>
> pr_devel("Creating xive for partition\n");
>
> + /* Already there ? */
> + if (kvm->arch.xive)
> + return -EEXIST;
> +
> xive = kvmppc_xive_get_device(kvm, type);
> if (!xive)
> return -ENOMEM;
> @@ -2014,12 +2018,6 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
> xive->kvm = kvm;
> mutex_init(&xive->lock);
>
> - /* Already there ? */
> - if (kvm->arch.xive)
> - ret = -EEXIST;
> - else
> - kvm->arch.xive = xive;
> -
> /* We use the default queue size set by the host */
> xive->q_order = xive_native_default_eq_shift();
> if (xive->q_order < PAGE_SHIFT)
> @@ -2040,6 +2038,7 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
> return ret;
>
> dev->private = xive;
> + kvm->arch.xive = xive;
> return 0;
> }
>
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index e9cbb42de424..84a354b90f60 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -1087,7 +1087,6 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
>
> xive->dev = dev;
> xive->kvm = kvm;
> - kvm->arch.xive = xive;
> mutex_init(&xive->mapping_lock);
> mutex_init(&xive->lock);
>
> @@ -1109,6 +1108,7 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
> return ret;
>
> dev->private = xive;
> + kvm->arch.xive = xive;
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-10 16:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-10 13:43 [PATCH 1/2] KVM: PPC: Book3S HV: XIVE: initialize private pointer when VPs are allocated Greg Kurz
2019-09-10 13:43 ` Greg Kurz
2019-09-10 13:44 ` [PATCH 2/2] KVM: PPC: Book3S HV: XIVE: Set kvm->arch.xive " Greg Kurz
2019-09-10 13:44 ` Greg Kurz
2019-09-10 13:49 ` Cédric Le Goater
2019-09-10 13:49 ` Cédric Le Goater
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.