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 <da...@gibson.dropbear.id.au> > --- > 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