Re: [PATCH v3 4/4] spapr: Forbid nested KVM-HV in pre-power9 compat mode
On Wed, Jul 15, 2020 at 01:14:42PM +0200, Greg Kurz wrote:
> On Mon, 13 Jul 2020 14:53:30 +1000
> David Gibson wrote:
>
> > On Fri, Jul 03, 2020 at 04:19:24PM +0200, Greg Kurz wrote:
> > > On Mon, 15 Jun 2020 11:20:31 +0200
> > > Greg Kurz wrote:
> > >
> > > > On Sat, 13 Jun 2020 17:18:04 +1000
> > > > David Gibson wrote:
> > > >
> > > > > On Thu, Jun 11, 2020 at 03:40:33PM +0200, Greg Kurz wrote:
> > > > > > Nested KVM-HV only works on POWER9.
> > > > > >
> > > > > > Signed-off-by: Greg Kurz
> > > > > > Reviewed-by: Laurent Vivier
> > > > >
> > > > > Hrm. I have mixed feelings about this. It does bring forward an
> > > > > error that we'd otherwise only discover when we try to load the kvm
> > > > > module in the guest.
> > > > >
> > > > > On the other hand, it's kind of a layering violation - really it's
> > > > > KVM's business to report what it can and can't do, rather than having
> > > > > qemu anticipate it.
> > > > >
> > > >
> > > > Agreed and it seems that we can probably get KVM to report that
> > > > already. I'll have closer look.
> > > >
> > >
> > > Checking the KVM_CAP_PPC_NESTED_HV extension only reports what the host
> > > supports. It can't reasonably take into account that we're going to
> > > switch vCPUs in some compat mode later on. KVM could possibly check
> > > that it has a vCPU in pre-power9 compat mode when we try to enable
> > > the capability and fail... but it would be a layering violation all
> > > the same. The KVM that doesn't like pre-power9 CPUs isn't the one in
> > > the host, it is the one in the guest, and it's not even directly
> > > related to the CPU type but to the MMU mode currently in use:
> > >
> > > long kvmhv_nested_init(void)
> > > {
> > > long int ptb_order;
> > > unsigned long ptcr;
> > > long rc;
> > >
> > > if (!kvmhv_on_pseries())
> > > return 0;
> > > ==> if (!radix_enabled())
> > > return -ENODEV;
> > >
> > > We cannot know either for sure the MMU mode the guest will run in
> > > when we enable the nested cap during the initial machine reset.
> > > So it seems we cannot do anything better than denylisting well
> > > known broken setups, in which case QEMU seems a better fit than
> > > KVM.
> > >
> > > Makes sense ?
> >
> > Yeah, good points.
> >
>
> So, should I just rebase/repost this or do you think of another
> way ?
Urgh... I've kind of forgotten the context while I've been away. So,
I guess repost and I'll take another look at them.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v3 4/4] spapr: Forbid nested KVM-HV in pre-power9 compat mode
On Mon, 13 Jul 2020 14:53:30 +1000
David Gibson wrote:
> On Fri, Jul 03, 2020 at 04:19:24PM +0200, Greg Kurz wrote:
> > On Mon, 15 Jun 2020 11:20:31 +0200
> > Greg Kurz wrote:
> >
> > > On Sat, 13 Jun 2020 17:18:04 +1000
> > > David Gibson wrote:
> > >
> > > > On Thu, Jun 11, 2020 at 03:40:33PM +0200, Greg Kurz wrote:
> > > > > Nested KVM-HV only works on POWER9.
> > > > >
> > > > > Signed-off-by: Greg Kurz
> > > > > Reviewed-by: Laurent Vivier
> > > >
> > > > Hrm. I have mixed feelings about this. It does bring forward an
> > > > error that we'd otherwise only discover when we try to load the kvm
> > > > module in the guest.
> > > >
> > > > On the other hand, it's kind of a layering violation - really it's
> > > > KVM's business to report what it can and can't do, rather than having
> > > > qemu anticipate it.
> > > >
> > >
> > > Agreed and it seems that we can probably get KVM to report that
> > > already. I'll have closer look.
> > >
> >
> > Checking the KVM_CAP_PPC_NESTED_HV extension only reports what the host
> > supports. It can't reasonably take into account that we're going to
> > switch vCPUs in some compat mode later on. KVM could possibly check
> > that it has a vCPU in pre-power9 compat mode when we try to enable
> > the capability and fail... but it would be a layering violation all
> > the same. The KVM that doesn't like pre-power9 CPUs isn't the one in
> > the host, it is the one in the guest, and it's not even directly
> > related to the CPU type but to the MMU mode currently in use:
> >
> > long kvmhv_nested_init(void)
> > {
> > long int ptb_order;
> > unsigned long ptcr;
> > long rc;
> >
> > if (!kvmhv_on_pseries())
> > return 0;
> > ==> if (!radix_enabled())
> > return -ENODEV;
> >
> > We cannot know either for sure the MMU mode the guest will run in
> > when we enable the nested cap during the initial machine reset.
> > So it seems we cannot do anything better than denylisting well
> > known broken setups, in which case QEMU seems a better fit than
> > KVM.
> >
> > Makes sense ?
>
> Yeah, good points.
>
So, should I just rebase/repost this or do you think of another
way ?
pgpGsqHEh38nu.pgp
Description: OpenPGP digital signature
Re: [PATCH v3 4/4] spapr: Forbid nested KVM-HV in pre-power9 compat mode
On Fri, Jul 03, 2020 at 04:19:24PM +0200, Greg Kurz wrote:
> On Mon, 15 Jun 2020 11:20:31 +0200
> Greg Kurz wrote:
>
> > On Sat, 13 Jun 2020 17:18:04 +1000
> > David Gibson wrote:
> >
> > > On Thu, Jun 11, 2020 at 03:40:33PM +0200, Greg Kurz wrote:
> > > > Nested KVM-HV only works on POWER9.
> > > >
> > > > Signed-off-by: Greg Kurz
> > > > Reviewed-by: Laurent Vivier
> > >
> > > Hrm. I have mixed feelings about this. It does bring forward an
> > > error that we'd otherwise only discover when we try to load the kvm
> > > module in the guest.
> > >
> > > On the other hand, it's kind of a layering violation - really it's
> > > KVM's business to report what it can and can't do, rather than having
> > > qemu anticipate it.
> > >
> >
> > Agreed and it seems that we can probably get KVM to report that
> > already. I'll have closer look.
> >
>
> Checking the KVM_CAP_PPC_NESTED_HV extension only reports what the host
> supports. It can't reasonably take into account that we're going to
> switch vCPUs in some compat mode later on. KVM could possibly check
> that it has a vCPU in pre-power9 compat mode when we try to enable
> the capability and fail... but it would be a layering violation all
> the same. The KVM that doesn't like pre-power9 CPUs isn't the one in
> the host, it is the one in the guest, and it's not even directly
> related to the CPU type but to the MMU mode currently in use:
>
> long kvmhv_nested_init(void)
> {
> long int ptb_order;
> unsigned long ptcr;
> long rc;
>
> if (!kvmhv_on_pseries())
> return 0;
> ==> if (!radix_enabled())
> return -ENODEV;
>
> We cannot know either for sure the MMU mode the guest will run in
> when we enable the nested cap during the initial machine reset.
> So it seems we cannot do anything better than denylisting well
> known broken setups, in which case QEMU seems a better fit than
> KVM.
>
> Makes sense ?
Yeah, good points.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v3 4/4] spapr: Forbid nested KVM-HV in pre-power9 compat mode
On Mon, 15 Jun 2020 11:20:31 +0200
Greg Kurz wrote:
> On Sat, 13 Jun 2020 17:18:04 +1000
> David Gibson wrote:
>
> > On Thu, Jun 11, 2020 at 03:40:33PM +0200, Greg Kurz wrote:
> > > Nested KVM-HV only works on POWER9.
> > >
> > > Signed-off-by: Greg Kurz
> > > Reviewed-by: Laurent Vivier
> >
> > Hrm. I have mixed feelings about this. It does bring forward an
> > error that we'd otherwise only discover when we try to load the kvm
> > module in the guest.
> >
> > On the other hand, it's kind of a layering violation - really it's
> > KVM's business to report what it can and can't do, rather than having
> > qemu anticipate it.
> >
>
> Agreed and it seems that we can probably get KVM to report that
> already. I'll have closer look.
>
Checking the KVM_CAP_PPC_NESTED_HV extension only reports what the host
supports. It can't reasonably take into account that we're going to
switch vCPUs in some compat mode later on. KVM could possibly check
that it has a vCPU in pre-power9 compat mode when we try to enable
the capability and fail... but it would be a layering violation all
the same. The KVM that doesn't like pre-power9 CPUs isn't the one in
the host, it is the one in the guest, and it's not even directly
related to the CPU type but to the MMU mode currently in use:
long kvmhv_nested_init(void)
{
long int ptb_order;
unsigned long ptcr;
long rc;
if (!kvmhv_on_pseries())
return 0;
==> if (!radix_enabled())
return -ENODEV;
We cannot know either for sure the MMU mode the guest will run in
when we enable the nested cap during the initial machine reset.
So it seems we cannot do anything better than denylisting well
known broken setups, in which case QEMU seems a better fit than
KVM.
Makes sense ?
> > Allowing POWER8 compat for an L2 is something we hope to have in the
> > fairly near future.
>
> Ok but I guess we don't want to start an L2 in compat POWER8 mode
> with cap-nested-hv=on, do we ?
>
> > Allowing POWER8 compat for L1, which is what this
> > covers, is, I'll admit, likely to never happen.
> >
> >
> > > ---
> > > HW/ppc/spapr_caps.c | 10 ++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> > > index 27cf2b38af27..dfe3b419daaa 100644
> > > --- a/hw/ppc/spapr_caps.c
> > > +++ b/hw/ppc/spapr_caps.c
> > > @@ -391,6 +391,8 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState
> > > *spapr,
> > > uint8_t val, Error **errp)
> > > {
> > > ERRP_AUTO_PROPAGATE();
> > > +PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> > > +
> > > if (!val) {
> > > /* capability disabled by default */
> > > return;
> > > @@ -400,6 +402,14 @@ static void
> > > cap_nested_kvm_hv_apply(SpaprMachineState *spapr,
> > > error_setg(errp, "No Nested KVM-HV support in TCG");
> > > error_append_hint(errp, "Try appending -machine
> > > cap-nested-hv=off\n");
> > > } else if (kvm_enabled()) {
> > > +if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
> > > + spapr->max_compat_pvr)) {
> > > +error_setg(errp, "Nested KVM-HV only supported on POWER9");
> > > +error_append_hint(errp,
> > > + "Try appending -machine
> > > max-cpu-compat=power9\n");
> > > +return;
> > > +}
> > > +
> > > if (!kvmppc_has_cap_nested_kvm_hv()) {
> > > error_setg(errp,
> > > "KVM implementation does not support Nested
> > > KVM-HV");
> > >
> > >
> >
>
pgpisRsErrqED.pgp
Description: OpenPGP digital signature
Re: [PATCH v3 4/4] spapr: Forbid nested KVM-HV in pre-power9 compat mode
On Mon, Jun 15, 2020 at 11:20:31AM +0200, Greg Kurz wrote: > On Sat, 13 Jun 2020 17:18:04 +1000 > David Gibson wrote: > > > On Thu, Jun 11, 2020 at 03:40:33PM +0200, Greg Kurz wrote: > > > Nested KVM-HV only works on POWER9. > > > > > > Signed-off-by: Greg Kurz > > > Reviewed-by: Laurent Vivier > > > > Hrm. I have mixed feelings about this. It does bring forward an > > error that we'd otherwise only discover when we try to load the kvm > > module in the guest. > > > > On the other hand, it's kind of a layering violation - really it's > > KVM's business to report what it can and can't do, rather than having > > qemu anticipate it. > > > > Agreed and it seems that we can probably get KVM to report that > already. I'll have closer look. > > > Allowing POWER8 compat for an L2 is something we hope to have in the > > fairly near future. > > Ok but I guess we don't want to start an L2 in compat POWER8 mode > with cap-nested-hv=on, do we ? Sorry, "L2" was misleading, I really mean L. Setting cap-nested-hv kind of implies there's a L which contradicts that. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [PATCH v3 4/4] spapr: Forbid nested KVM-HV in pre-power9 compat mode
On Sat, 13 Jun 2020 17:18:04 +1000
David Gibson wrote:
> On Thu, Jun 11, 2020 at 03:40:33PM +0200, Greg Kurz wrote:
> > Nested KVM-HV only works on POWER9.
> >
> > Signed-off-by: Greg Kurz
> > Reviewed-by: Laurent Vivier
>
> Hrm. I have mixed feelings about this. It does bring forward an
> error that we'd otherwise only discover when we try to load the kvm
> module in the guest.
>
> On the other hand, it's kind of a layering violation - really it's
> KVM's business to report what it can and can't do, rather than having
> qemu anticipate it.
>
Agreed and it seems that we can probably get KVM to report that
already. I'll have closer look.
> Allowing POWER8 compat for an L2 is something we hope to have in the
> fairly near future.
Ok but I guess we don't want to start an L2 in compat POWER8 mode
with cap-nested-hv=on, do we ?
> Allowing POWER8 compat for L1, which is what this
> covers, is, I'll admit, likely to never happen.
>
>
> > ---
> > HW/ppc/spapr_caps.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> > index 27cf2b38af27..dfe3b419daaa 100644
> > --- a/hw/ppc/spapr_caps.c
> > +++ b/hw/ppc/spapr_caps.c
> > @@ -391,6 +391,8 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState
> > *spapr,
> > uint8_t val, Error **errp)
> > {
> > ERRP_AUTO_PROPAGATE();
> > +PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> > +
> > if (!val) {
> > /* capability disabled by default */
> > return;
> > @@ -400,6 +402,14 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState
> > *spapr,
> > error_setg(errp, "No Nested KVM-HV support in TCG");
> > error_append_hint(errp, "Try appending -machine
> > cap-nested-hv=off\n");
> > } else if (kvm_enabled()) {
> > +if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
> > + spapr->max_compat_pvr)) {
> > +error_setg(errp, "Nested KVM-HV only supported on POWER9");
> > +error_append_hint(errp,
> > + "Try appending -machine
> > max-cpu-compat=power9\n");
> > +return;
> > +}
> > +
> > if (!kvmppc_has_cap_nested_kvm_hv()) {
> > error_setg(errp,
> > "KVM implementation does not support Nested
> > KVM-HV");
> >
> >
>
pgp40aswN_k78.pgp
Description: OpenPGP digital signature
Re: [PATCH v3 4/4] spapr: Forbid nested KVM-HV in pre-power9 compat mode
On Thu, Jun 11, 2020 at 03:40:33PM +0200, Greg Kurz wrote:
> Nested KVM-HV only works on POWER9.
>
> Signed-off-by: Greg Kurz
> Reviewed-by: Laurent Vivier
Hrm. I have mixed feelings about this. It does bring forward an
error that we'd otherwise only discover when we try to load the kvm
module in the guest.
On the other hand, it's kind of a layering violation - really it's
KVM's business to report what it can and can't do, rather than having
qemu anticipate it.
Allowing POWER8 compat for an L2 is something we hope to have in the
fairly near future. Allowing POWER8 compat for L1, which is what this
covers, is, I'll admit, likely to never happen.
> ---
> HW/ppc/spapr_caps.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 27cf2b38af27..dfe3b419daaa 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -391,6 +391,8 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState
> *spapr,
> uint8_t val, Error **errp)
> {
> ERRP_AUTO_PROPAGATE();
> +PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> +
> if (!val) {
> /* capability disabled by default */
> return;
> @@ -400,6 +402,14 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState
> *spapr,
> error_setg(errp, "No Nested KVM-HV support in TCG");
> error_append_hint(errp, "Try appending -machine
> cap-nested-hv=off\n");
> } else if (kvm_enabled()) {
> +if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
> + spapr->max_compat_pvr)) {
> +error_setg(errp, "Nested KVM-HV only supported on POWER9");
> +error_append_hint(errp,
> + "Try appending -machine
> max-cpu-compat=power9\n");
> +return;
> +}
> +
> if (!kvmppc_has_cap_nested_kvm_hv()) {
> error_setg(errp,
> "KVM implementation does not support Nested KVM-HV");
>
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[PATCH v3 4/4] spapr: Forbid nested KVM-HV in pre-power9 compat mode
Nested KVM-HV only works on POWER9.
Signed-off-by: Greg Kurz
Reviewed-by: Laurent Vivier
---
hw/ppc/spapr_caps.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 27cf2b38af27..dfe3b419daaa 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -391,6 +391,8 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState
*spapr,
uint8_t val, Error **errp)
{
ERRP_AUTO_PROPAGATE();
+PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+
if (!val) {
/* capability disabled by default */
return;
@@ -400,6 +402,14 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState
*spapr,
error_setg(errp, "No Nested KVM-HV support in TCG");
error_append_hint(errp, "Try appending -machine cap-nested-hv=off\n");
} else if (kvm_enabled()) {
+if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
+ spapr->max_compat_pvr)) {
+error_setg(errp, "Nested KVM-HV only supported on POWER9");
+error_append_hint(errp,
+ "Try appending -machine
max-cpu-compat=power9\n");
+return;
+}
+
if (!kvmppc_has_cap_nested_kvm_hv()) {
error_setg(errp,
"KVM implementation does not support Nested KVM-HV");
