[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/1] target-ppc: Implement rtas_get_sysparm(P
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/1] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO) |
Date: |
Mon, 9 Nov 2015 16:01:49 +1100 |
User-agent: |
Mutt/1.5.23 (2015-06-09) |
On Mon, Nov 09, 2015 at 12:57:48PM +1100, Alexey Kardashevskiy wrote:
> On 11/05/2015 10:06 AM, Sukadev Bhattiprolu wrote:
> >Implement RTAS_SYSPARM_PROCESSOR_MODULE_INFO parameter to rtas_get_sysparm()
> >call in qemu. This call returns the processor module (socket), chip and core
> >information as specified in section 7.3.16.18 of PAPR v2.7.
> >
> >We walk the /proc/device-tree to determine the number of chips, cores and
> >modules in the _host_ system and return this info to the guest application
> >that makes the rtas_get_sysparm() call.
> >
> >We currently hard code the number of module_types to 1, but we should
> >determine
> >that dynamically somehow later.
> >
> >Thanks to input from Nishanth Aravamudan and Alexey Kardashevsk.
>
> "iy" is missing :)
>
>
> >
> >Signed-off-by: Sukadev Bhattiprolu <address@hidden>
> >---
> >Changelog[v2]:
> > [Alexey Kardashevsk] Use existing interfaces like ldl_be_p(),
> > stw_be_phys(), g_hash_table_new_full(), error_report()
> > rather
> > than re-inventing; fix indentation, function prottypes etc;
> > Drop the fts() interface (which doesn't seem to be available
> > on mingw32/mingw64) and use opendir() to walk specific
> > directories in the directory tree.
> >---
> > hw/ppc/Makefile.objs | 1 +
> > hw/ppc/spapr_rtas.c | 35 +++++++
> > hw/ppc/spapr_rtas_modinfo.c | 230
> > ++++++++++++++++++++++++++++++++++++++++++++
> > hw/ppc/spapr_rtas_modinfo.h | 12 +++
> > include/hw/ppc/spapr.h | 1 +
> > 5 files changed, 279 insertions(+)
> > create mode 100644 hw/ppc/spapr_rtas_modinfo.c
> > create mode 100644 hw/ppc/spapr_rtas_modinfo.h
> >
> >diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> >index c1ffc77..57c6b02 100644
> >--- a/hw/ppc/Makefile.objs
> >+++ b/hw/ppc/Makefile.objs
> >@@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o
> > obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
> > obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> > obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
> >+obj-$(CONFIG_PSERIES) += spapr_rtas_modinfo.o
> > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> > obj-y += spapr_pci_vfio.o
> > endif
> >diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >index 34b12a3..41fd8a6 100644
> >--- a/hw/ppc/spapr_rtas.c
> >+++ b/hw/ppc/spapr_rtas.c
> >@@ -34,6 +34,8 @@
> > #include "hw/ppc/spapr.h"
> > #include "hw/ppc/spapr_vio.h"
> > #include "qapi-event.h"
> >+
> >+#include "spapr_rtas_modinfo.h"
> > #include "hw/boards.h"
> >
> > #include <libfdt.h>
> >@@ -240,6 +242,39 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU
> >*cpu,
> > target_ulong ret = RTAS_OUT_SUCCESS;
> >
> > switch (parameter) {
> >+ case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: {
> >+ int i;
> >+ int offset = 0;
> >+ int size;
>
> Nit - could be one line.
>
>
> >+ struct rtas_module_info modinfo;
> >+
> >+ if (rtas_get_module_info(&modinfo)) {
> >+ break;
>
>
> @ret will be still 0 in this case, set @ret to RTAS_OUT_HW_ERROR here.
>
> Also, rtas_ibm_set_system_parameter() must return RTAS_OUT_NOT_AUTHORIZED on
> RTAS_SYSPARM_PROCESSOR_MODULE_INFO, as PAPR says.
>
>
> >+ }
> >+
> >+ size = sizeof(modinfo);
> >+ size += (modinfo.module_types - 1) * sizeof(struct
> >rtas_socket_info);
> >+
> >+ stw_be_phys(&address_space_memory, buffer+offset, size);
> >+ offset += 2;
> >+
> >+ stw_be_phys(&address_space_memory, buffer+offset,
> >modinfo.module_types);
> >+ offset += 2;
> >+
> >+ for (i = 0; i < modinfo.module_types; i++) {
> >+ stw_be_phys(&address_space_memory, buffer+offset,
> >+ modinfo.si[i].sockets);
>
>
> checkpatch.pl does not warn on this but new lines start under opening brace
> in the previous line.
>
> In terms on vim, it would be:
> set expandtab
> set tabstop=4
> set shiftwidth=4
> set cino=:0,(0
>
>
>
> >+ offset += 2;
> >+ stw_be_phys(&address_space_memory, buffer+offset,
> >+ modinfo.si[i].chips);
> >+ offset += 2;
> >+ stw_be_phys(&address_space_memory, buffer+offset,
> >+ modinfo.si[i].cores_per_chip);
> >+ offset += 2;
> >+ }
> >+ break;
> >+ }
> >+
> > case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
> > char *param_val = g_strdup_printf("MaxEntCap=%d,"
> > "DesMem=%llu,"
> >diff --git a/hw/ppc/spapr_rtas_modinfo.c b/hw/ppc/spapr_rtas_modinfo.c
> >new file mode 100644
> >index 0000000..068fc2c
> >--- /dev/null
> >+++ b/hw/ppc/spapr_rtas_modinfo.c
> >@@ -0,0 +1,230 @@
> >+/*
> >+ * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System
> >Emulator
> >+ *
> >+ * Hypercall based emulated RTAS
>
>
> This is a description of hw/ppc/spapr_rtas.c, not of the new file.
>
> Not sure the new code deserves a separate file, I'd either:
> - add it into hw/ppc/spapr_rtas.c OR
> - move rtas_ibm_get_system_parameter() + rtas_ibm_set_system_parameter() to
> a separate file (let's call it hw/ppc/spapr_rtas_syspar.c) and add this new
> parameter there as there will be new parameters in the future anyway.
>
> But I'll leave to the maintainer (David, hi :) ).
Actually it probably should go in target-ppc/kvm.c. Looking up things
on the host in this way only makes sense for KVM, and we have other
code in there that looks in the host /proc/device-tree.
--
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: [Qemu-devel] [PATCH v2 1/1] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO), David Gibson, 2015/11/09