On Wed, Oct 12, 2016 at 06:13:50PM -0500, Michael Roth wrote: > Currently we access individual bytes of an option vector via > ldub_phys() to test for the presence of a particular capability > within that byte. Currently this is only done for the "dynamic > reconfiguration memory" capability bit. If that bit is present, > we pass a boolean value to spapr_h_cas_compose_response() > to generate a modified device tree segment with the additional > properties required to enable this functionality. > > As more capability bits are added, will would need to modify the > code to add additional option vector accesses and extend the > param list for spapr_h_cas_compose_response() to include similar > boolean values for these parameters. > > Avoid this by switching to spapr_ovec_* helpers so we can do all > the parsing in one shot and then test for these additional bits > within spapr_h_cas_compose_response() directly. > > Cc: Bharata B Rao <bhar...@linux.vnet.ibm.com> > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr.c | 10 ++++++--
> hw/ppc/spapr_hcall.c | 56
> ++++++++++++---------------------------------
> include/hw/ppc/spapr.h | 5 +++-
> include/hw/ppc/spapr_ovec.h | 3 +++
> 4 files changed, 30 insertions(+), 44 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 03e3803..934d6b2 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -856,7 +856,7 @@ out:
>
> int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
> target_ulong addr, target_ulong size,
> - bool cpu_update, bool memory_update)
> + bool cpu_update)
> {
> void *fdt, *fdt_skel;
> sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
> @@ -880,7 +880,8 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
> }
>
> /* Generate ibm,dynamic-reconfiguration-memory node if required */
> - if (memory_update && smc->dr_lmb_enabled) {
> + if (spapr_ovec_test(spapr->ov5_cas, OV5_DRCONF_MEMORY)) {
> + g_assert(smc->dr_lmb_enabled);
> _FDT((spapr_populate_drconf_memory(spapr, fdt)));
> }
>
> @@ -1769,7 +1770,12 @@ static void ppc_spapr_init(MachineState *machine)
> DIV_ROUND_UP(max_cpus * smt, smp_threads),
> XICS_IRQS_SPAPR, &error_fatal);
>
> + /* Set up containers for ibm,client-set-architecture negotiated options
> */
> + spapr->ov5 = spapr_ovec_new();
> + spapr->ov5_cas = spapr_ovec_new();
> +
> if (smc->dr_lmb_enabled) {
> + spapr_ovec_set(spapr->ov5, OV5_DRCONF_MEMORY);
> spapr_validate_node_memory(machine, &error_fatal);
> }
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index c5e7e8c..f1d081b 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -11,6 +11,7 @@
> #include "trace.h"
> #include "sysemu/kvm.h"
> #include "kvm_ppc.h"
> +#include "hw/ppc/spapr_ovec.h"
>
> struct SPRSyncState {
> int spr;
> @@ -880,32 +881,6 @@ static target_ulong h_set_mode(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> return ret;
> }
>
> -/*
> - * Return the offset to the requested option vector @vector in the
> - * option vector table @table.
> - */
> -static target_ulong cas_get_option_vector(int vector, target_ulong table)
> -{
> - int i;
> - char nr_vectors, nr_entries;
> -
> - if (!table) {
> - return 0;
> - }
> -
> - nr_vectors = (ldl_phys(&address_space_memory, table) >> 24) + 1;
> - if (!vector || vector > nr_vectors) {
> - return 0;
> - }
> - table++; /* skip nr option vectors */
> -
> - for (i = 0; i < vector - 1; i++) {
> - nr_entries = ldl_phys(&address_space_memory, table) >> 24;
> - table += nr_entries + 2;
> - }
> - return table;
> -}
> -
> typedef struct {
> uint32_t cpu_version;
> Error *err;
> @@ -961,23 +936,21 @@ static void cas_handle_compat_cpu(PowerPCCPUClass *pcc,
> uint32_t pvr,
> }
> }
>
> -#define OV5_DRCONF_MEMORY 0x20
> -
> static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
> sPAPRMachineState *spapr,
> target_ulong opcode,
> target_ulong *args)
> {
> target_ulong list = ppc64_phys_to_real(args[0]);
> - target_ulong ov_table, ov5;
> + target_ulong ov_table;
> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_);
> CPUState *cs;
> - bool cpu_match = false, cpu_update = true, memory_update = false;
> + bool cpu_match = false, cpu_update = true;
> unsigned old_cpu_version = cpu_->cpu_version;
> unsigned compat_lvl = 0, cpu_version = 0;
> unsigned max_lvl = get_compat_level(cpu_->max_compat);
> int counter;
> - char ov5_byte2;
> + sPAPROptionVector *ov5_guest;
>
> /* Parse PVR list */
> for (counter = 0; counter < 512; ++counter) {
> @@ -1033,19 +1006,20 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu_,
> /* For the future use: here @ov_table points to the first option vector
> */
> ov_table = list;
>
> - ov5 = cas_get_option_vector(5, ov_table);
> - if (!ov5) {
> - return H_SUCCESS;
> - }
> + ov5_guest = spapr_ovec_parse_vector(ov_table, 5);
>
> - /* @list now points to OV 5 */
> - ov5_byte2 = ldub_phys(&address_space_memory, ov5 + 2);
> - if (ov5_byte2 & OV5_DRCONF_MEMORY) {
> - memory_update = true;
> - }
> + /* NOTE: there are actually a number of ov5 bits where input from the
> + * guest is always zero, and the platform/QEMU enables them independently
> + * of guest input. To model these properly we'd want some sort of mask,
> + * but since they only currently apply to memory migration as defined
> + * by LoPAPR 1.1, 14.5.4.8, which QEMU doesn't implement, we don't need
> + * to worry about this.
> + */
> + spapr_ovec_intersect(spapr->ov5_cas, spapr->ov5, ov5_guest);
> + spapr_ovec_cleanup(ov5_guest);
>
> if (spapr_h_cas_compose_response(spapr, args[1], args[2],
> - cpu_update, memory_update)) {
> + cpu_update)) {
> qemu_system_reset_request();
> }
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 39dadaa..6c20d28 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -6,6 +6,7 @@
> #include "hw/ppc/xics.h"
> #include "hw/ppc/spapr_drc.h"
> #include "hw/mem/pc-dimm.h"
> +#include "hw/ppc/spapr_ovec.h"
>
> struct VIOsPAPRBus;
> struct sPAPRPHBState;
> @@ -66,6 +67,8 @@ struct sPAPRMachineState {
> uint64_t rtc_offset; /* Now used only during incoming migration */
> struct PPCTimebase tb;
> bool has_graphics;
> + sPAPROptionVector *ov5;
> + sPAPROptionVector *ov5_cas;
>
> uint32_t check_exception_irq;
> Notifier epow_notifier;
> @@ -577,7 +580,7 @@ void spapr_events_init(sPAPRMachineState *sm);
> void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq);
> int spapr_h_cas_compose_response(sPAPRMachineState *sm,
> target_ulong addr, target_ulong size,
> - bool cpu_update, bool memory_update);
> + bool cpu_update);
> sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn);
> void spapr_tce_table_enable(sPAPRTCETable *tcet,
> uint32_t page_shift, uint64_t bus_offset,
> diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
> index fba2d98..09afd59 100644
> --- a/include/hw/ppc/spapr_ovec.h
> +++ b/include/hw/ppc/spapr_ovec.h
> @@ -42,6 +42,9 @@ typedef struct sPAPROptionVector sPAPROptionVector;
>
> #define OV_BIT(byte, bit) ((byte - 1) * BITS_PER_BYTE + bit)
>
> +/* option vector 5 */
> +#define OV5_DRCONF_MEMORY OV_BIT(2, 2)
> +
> /* interfaces */
> sPAPROptionVector *spapr_ovec_new(void);
> sPAPROptionVector *spapr_ovec_clone(sPAPROptionVector *ov_orig);
--
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
