[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [for-2.9 2/5] pseries: Stubs for HPT resizing
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [for-2.9 2/5] pseries: Stubs for HPT resizing |
Date: |
Fri, 9 Dec 2016 09:18:51 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 |
On 09.12.2016 03:23, David Gibson wrote:
> This introduces stub implementations of the H_RESIZE_HPT_PREPARE and
> H_RESIZE_HPT_COMMIT hypercalls which we hope to add in a PAPR
> extension to allow run time resizing of a guest's hash page table. It
> also adds a new machine property for controlling whether this new
> facility is available, and logic to check that against availability
> with KVM (only supported with KVM PR for now).
>
> Finally, it adds a new string to the hypertas property in the device
> tree, advertising to the guest the availability of the HPT resizing
> hypercalls. This is a tentative suggested value, and would need to be
> standardized by PAPR before being merged.
>
> Signed-off-by: David Gibson <address@hidden>
> ---
> hw/ppc/spapr.c | 66
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/ppc/spapr_hcall.c | 36 +++++++++++++++++++++++++++
> hw/ppc/trace-events | 2 ++
> include/hw/ppc/spapr.h | 11 +++++++++
> target-ppc/kvm.c | 26 ++++++++++++++++++++
> target-ppc/kvm_ppc.h | 5 ++++
> 6 files changed, 146 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0f25e83..ecb0822 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -760,6 +760,11 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void
> *fdt)
> if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
> add_str(hypertas, "hcall-multi-tce");
> }
> +
> + if (spapr->resize_hpt != SPAPR_RESIZE_HPT_DISABLED) {
> + add_str(hypertas, "hcall-hpt-resize");
> + }
> +
> _FDT(fdt_setprop(fdt, rtas, "ibm,hypertas-functions",
> hypertas->str, hypertas->len));
> g_string_free(hypertas, TRUE);
> @@ -1839,11 +1844,31 @@ static void ppc_spapr_init(MachineState *machine)
> long load_limit, fw_size;
> char *filename;
> int smt = kvmppc_smt_threads();
> + Error *resize_hpt_err = NULL;
>
> msi_nonbroken = true;
>
> QLIST_INIT(&spapr->phbs);
>
> + /* Check HPT resizing availability */
> + kvmppc_check_papr_resize_hpt(&resize_hpt_err);
> + if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DEFAULT) {
> + if (resize_hpt_err) {
> + spapr->resize_hpt = SPAPR_RESIZE_HPT_DISABLED;
> + error_free(resize_hpt_err);
> + resize_hpt_err = NULL;
> + } else {
> + spapr->resize_hpt = smc->resize_hpt_default;
> + }
> + }
> +
> + assert(spapr->resize_hpt != SPAPR_RESIZE_HPT_DEFAULT);
> +
> + if ((spapr->resize_hpt != SPAPR_RESIZE_HPT_DISABLED) && resize_hpt_err) {
> + error_report_err(resize_hpt_err);
> + exit(1);
> + }
The logic here is IMHO a little bit hard to understand. Why do you need
the SPAPR_RESIZE_HPT_DEFAULT state here at all? Wouldn't it be
sufficient to start with SPAPR_RESIZE_HPT_DISABLED by default and to
only enable it when the extension is available? ... at least some
explaining comments in the code about SPAPR_RESIZE_HPT_DEFAULT could
help here.
> @@ -2236,6 +2261,40 @@ static void spapr_set_modern_hotplug_events(Object
> *obj, bool value,
> spapr->use_hotplug_event_source = value;
> }
>
> +static char *spapr_get_resize_hpt(Object *obj, Error **errp)
> +{
> + sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> +
> + switch (spapr->resize_hpt) {
> + case SPAPR_RESIZE_HPT_DEFAULT:
> + return g_strdup("default");
> + case SPAPR_RESIZE_HPT_DISABLED:
> + return g_strdup("disabled");
> + case SPAPR_RESIZE_HPT_ENABLED:
> + return g_strdup("enabled");
> + case SPAPR_RESIZE_HPT_REQUIRED:
> + return g_strdup("required");
> + }
> + assert(0);
> +}
> +
> +static void spapr_set_resize_hpt(Object *obj, const char *value, Error
> **errp)
> +{
> + sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> +
> + if (strcmp(value, "default") == 0) {
> + spapr->resize_hpt = SPAPR_RESIZE_HPT_DEFAULT;
> + } else if (strcmp(value, "disabled") == 0) {
> + spapr->resize_hpt = SPAPR_RESIZE_HPT_DISABLED;
> + } else if (strcmp(value, "enabled") == 0) {
> + spapr->resize_hpt = SPAPR_RESIZE_HPT_ENABLED;
> + } else if (strcmp(value, "required") == 0) {
> + spapr->resize_hpt = SPAPR_RESIZE_HPT_REQUIRED;
> + } else {
> + error_setg(errp, "Bad value for \"resize-hpt\" property");
> + }
> +}
> +
> static void spapr_machine_initfn(Object *obj)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> @@ -2256,6 +2315,12 @@ static void spapr_machine_initfn(Object *obj)
> " place of standard EPOW events when
> possible"
> " (required for memory hot-unplug
> support)",
> NULL);
> +
> + object_property_add_str(obj, "resize-hpt",
> + spapr_get_resize_hpt, spapr_set_resize_hpt,
> NULL);
> + object_property_set_description(obj, "resize-hpt",
> + "Resizing of the Hash Page Table
> (enabled, disabled, required)",
> + NULL);
... and "default" ?
> }
>
> static void spapr_machine_finalizefn(Object *obj)
> @@ -2707,6 +2772,7 @@ static void spapr_machine_class_init(ObjectClass *oc,
> void *data)
>
> smc->dr_lmb_enabled = true;
> smc->tcg_default_cpu = "POWER8";
> + smc->resize_hpt_default = SPAPR_RESIZE_HPT_DISABLED;
> mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
> fwc->get_dev_path = spapr_get_fw_dev_path;
> nc->nmi_monitor_handler = spapr_nmi;
...
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 15e12f3..dcd8cef 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -22,6 +22,7 @@
> #include <linux/kvm.h>
>
> #include "qemu-common.h"
> +#include "qapi/error.h"
> #include "qemu/error-report.h"
> #include "cpu.h"
> #include "qemu/timer.h"
> @@ -2269,6 +2270,7 @@ int kvmppc_reset_htab(int shift_hint)
> /* Full emulation, tell caller to allocate htab itself */
> return 0;
> }
> +
Unnecessary white space change.
> if (kvm_check_extension(kvm_state, KVM_CAP_PPC_ALLOC_HTAB)) {
> int ret;
> ret = kvm_vm_ioctl(kvm_state, KVM_PPC_ALLOCATE_HTAB, &shift);
> @@ -2672,3 +2674,27 @@ int kvmppc_enable_hwrng(void)
>
> return kvmppc_enable_hcall(kvm_state, H_RANDOM);
> }
> +
> +void kvmppc_check_papr_resize_hpt(Error **errp)
> +{
> + if (!kvm_enabled()) {
> + return;
> + }
> +
> + /* TODO: Check specific capabilities for HPT resize aware host kernels */
> +
> + /*
> + * It's tempting to try to check directly if the HPT is under our
> + * control or KVM's, which is what's really relevant here.
> + * Unfortunately, in order to correctly size the HPT, we need to
> + * know if we can do resizing, _before_ we attempt to allocate it
> + * with KVM. Before that point, we don't officially know whether
> + * we'll control the HPT or not. So we have to use a fallback
> + * test for PR vs HV KVM to predict that.
> + */
> + if (kvmppc_is_pr(kvm_state)) {
> + return;
> + }
Couldn't we get HPT resizing on KVM-PR one day, too?
> + error_setg(errp, "Hash page table resizing not available with this KVM
> version");
> +}
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 841a29b..3e852ba 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -59,6 +59,7 @@ bool kvmppc_has_cap_htm(void);
> int kvmppc_enable_hwrng(void);
> int kvmppc_put_books_sregs(PowerPCCPU *cpu);
> PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> +void kvmppc_check_papr_resize_hpt(Error **errp);
>
> #else
>
> @@ -270,6 +271,10 @@ static inline PowerPCCPUClass
> *kvm_ppc_get_host_cpu_class(void)
> return NULL;
> }
>
> +static inline void kvmppc_check_papr_resize_hpt(Error **errp)
> +{
> + return;
> +}
> #endif
Thomas
[Qemu-devel] [for-2.9 4/5] pseries: Enable HPT resizing for 2.9, David Gibson, 2016/12/08
[Qemu-devel] [for-2.9 5/5] pseries: Use smaller default hash page tables when guest can resize, David Gibson, 2016/12/08