[Qemu-devel] [PATCH v3] spapr: compute default value of "hpt-max-page-size" later

[Qemu-devel] [PATCH v3] spapr: compute default value of "hpt-max-page-size" later

[Greg Kurz]

It is currently not possible to run a pseries-2.12 or older machine
with HV KVM. QEMU prints the following and exits right away.

qemu-system-ppc64: KVM doesn't support for base page shift 34

The "hpt-max-page-size" capability was recently added to spapr to hide
host configuration details from HPT mode guests. Its default value for
newer machine types is 64k.

For backwards compatibility, pseries-2.12 and older machine types need
a different value. This is handled as usual in a class init function.
The default value is 16G, ie, all page sizes supported by POWER7 and
newer CPUs, but HV KVM requires guest pages to be hpa contiguous as
well as gpa contiguous. The default value is the page size used to
back the guest RAM in this case.

Unfortunately kvmppc_hpt_needs_host_contiguous_pages()->kvm_enabled() is
called way before KVM init and returns false, even if the user requested
KVM. We thus end up selecting 16G, which isn't supported by HV KVM. The
default value must be set during machine init, because we can safely
assume that KVM is initialized at this point.

We fix this by moving the logic to default_caps_with_cpu(). Since the
user cannot pass cap-hpt-max-page-size=0, we set the default to 0 in
the pseries-2.12 class init function and use that as a flag to do the
real work.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
v3: - moved logic to default_caps_with_cpu()

v2: - moved logic under spapr_caps_init()
---
 hw/ppc/spapr.c      |   13 ++++++-------
 hw/ppc/spapr_caps.c |   13 +++++++++++++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b2baec026ff4..062d9dc346d4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4095,17 +4095,16 @@ static void spapr_machine_2_12_instance_options(MachineState *machine)
 static void spapr_machine_2_12_class_options(MachineClass *mc)
 {
     sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
-    uint8_t mps;
 
     spapr_machine_3_0_class_options(mc);
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_12);
 
-    if (kvmppc_hpt_needs_host_contiguous_pages()) {
-        mps = ctz64(qemu_getrampagesize());
-    } else {
-        mps = 34; /* allow everything up to 16GiB, i.e. everything */
-    }
-    smc->default_caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = mps;
+    /* We depend on kvm_enabled() to choose a default value for the
+     * hpt-max-page-size capability. Of course we can't do it here
+     * because this is too early and the HW accelerator isn't initialzed
+     * yet. Postpone this to machine init (see default_caps_with_cpu()).
+     */
+    smc->default_caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = 0;
 }
 
 DEFINE_SPAPR_MACHINE(2_12, "2.12", false);
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 62663ebdf51a..aa605cea9108 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -465,6 +465,19 @@ static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
         caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN;
     }
 
+    /* This is for pseries-2.12 and older */
+    if (smc->default_caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] == 0) {
+        uint8_t mps;
+
+        if (kvmppc_hpt_needs_host_contiguous_pages()) {
+            mps = ctz64(qemu_getrampagesize());
+        } else {
+            mps = 34; /* allow everything up to 16GiB, i.e. everything */
+        }
+
+        caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = mps;
+    }
+
     return caps;
 }
 

Re: [Qemu-devel] [PATCH v3] spapr: compute default value of "hpt-max-page-size" later

[Eduardo Habkost]

On Mon, Jul 02, 2018 at 10:54:56AM +0200, Greg Kurz wrote:
> It is currently not possible to run a pseries-2.12 or older machine
> with HV KVM. QEMU prints the following and exits right away.
> 
> qemu-system-ppc64: KVM doesn't support for base page shift 34
> 
> The "hpt-max-page-size" capability was recently added to spapr to hide
> host configuration details from HPT mode guests. Its default value for
> newer machine types is 64k.
> 
> For backwards compatibility, pseries-2.12 and older machine types need
> a different value. This is handled as usual in a class init function.
> The default value is 16G, ie, all page sizes supported by POWER7 and
> newer CPUs, but HV KVM requires guest pages to be hpa contiguous as
> well as gpa contiguous. The default value is the page size used to
> back the guest RAM in this case.
> 
> Unfortunately kvmppc_hpt_needs_host_contiguous_pages()->kvm_enabled() is
> called way before KVM init and returns false, even if the user requested
> KVM. We thus end up selecting 16G, which isn't supported by HV KVM. The
> default value must be set during machine init, because we can safely
> assume that KVM is initialized at this point.
> 
> We fix this by moving the logic to default_caps_with_cpu(). Since the
> user cannot pass cap-hpt-max-page-size=0, we set the default to 0 in
> the pseries-2.12 class init function and use that as a flag to do the
> real work.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3] spapr: compute default value of "hpt-max-page-size" later

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3985 bytes --]

On Mon, Jul 02, 2018 at 10:54:56AM +0200, Greg Kurz wrote:
> It is currently not possible to run a pseries-2.12 or older machine
> with HV KVM. QEMU prints the following and exits right away.
> 
> qemu-system-ppc64: KVM doesn't support for base page shift 34
> 
> The "hpt-max-page-size" capability was recently added to spapr to hide
> host configuration details from HPT mode guests. Its default value for
> newer machine types is 64k.
> 
> For backwards compatibility, pseries-2.12 and older machine types need
> a different value. This is handled as usual in a class init function.
> The default value is 16G, ie, all page sizes supported by POWER7 and
> newer CPUs, but HV KVM requires guest pages to be hpa contiguous as
> well as gpa contiguous. The default value is the page size used to
> back the guest RAM in this case.
> 
> Unfortunately kvmppc_hpt_needs_host_contiguous_pages()->kvm_enabled() is
> called way before KVM init and returns false, even if the user requested
> KVM. We thus end up selecting 16G, which isn't supported by HV KVM. The
> default value must be set during machine init, because we can safely
> assume that KVM is initialized at this point.
> 
> We fix this by moving the logic to default_caps_with_cpu(). Since the
> user cannot pass cap-hpt-max-page-size=0, we set the default to 0 in
> the pseries-2.12 class init function and use that as a flag to do the
> real work.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-3.0, thanks.

> ---
> v3: - moved logic to default_caps_with_cpu()
> 
> v2: - moved logic under spapr_caps_init()
> ---
>  hw/ppc/spapr.c      |   13 ++++++-------
>  hw/ppc/spapr_caps.c |   13 +++++++++++++
>  2 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b2baec026ff4..062d9dc346d4 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4095,17 +4095,16 @@ static void spapr_machine_2_12_instance_options(MachineState *machine)
>  static void spapr_machine_2_12_class_options(MachineClass *mc)
>  {
>      sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> -    uint8_t mps;
>  
>      spapr_machine_3_0_class_options(mc);
>      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_12);
>  
> -    if (kvmppc_hpt_needs_host_contiguous_pages()) {
> -        mps = ctz64(qemu_getrampagesize());
> -    } else {
> -        mps = 34; /* allow everything up to 16GiB, i.e. everything */
> -    }
> -    smc->default_caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = mps;
> +    /* We depend on kvm_enabled() to choose a default value for the
> +     * hpt-max-page-size capability. Of course we can't do it here
> +     * because this is too early and the HW accelerator isn't initialzed
> +     * yet. Postpone this to machine init (see default_caps_with_cpu()).
> +     */
> +    smc->default_caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = 0;
>  }
>  
>  DEFINE_SPAPR_MACHINE(2_12, "2.12", false);
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 62663ebdf51a..aa605cea9108 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -465,6 +465,19 @@ static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
>          caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN;
>      }
>  
> +    /* This is for pseries-2.12 and older */
> +    if (smc->default_caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] == 0) {
> +        uint8_t mps;
> +
> +        if (kvmppc_hpt_needs_host_contiguous_pages()) {
> +            mps = ctz64(qemu_getrampagesize());
> +        } else {
> +            mps = 34; /* allow everything up to 16GiB, i.e. everything */
> +        }
> +
> +        caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = mps;
> +    }
> +
>      return caps;
>  }
>  
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]