From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58143) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw0s2-0000dP-U5 for qemu-devel@nongnu.org; Mon, 09 Nov 2015 23:49:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zw0rz-0007wE-Kj for qemu-devel@nongnu.org; Mon, 09 Nov 2015 23:49:26 -0500 Received: from e18.ny.us.ibm.com ([129.33.205.208]:47406) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw0rz-0007vy-Hm for qemu-devel@nongnu.org; Mon, 09 Nov 2015 23:49:23 -0500 Received: from localhost by e18.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 9 Nov 2015 23:49:22 -0500 Date: Mon, 9 Nov 2015 20:46:40 -0800 From: Sukadev Bhattiprolu Message-ID: <20151110044640.GA32368@us.ibm.com> References: <1446678366-15082-1-git-send-email-sukadev@linux.vnet.ibm.com> <563FFD9C.7070407@ozlabs.ru> <20151110035757.GA20030@us.ibm.com> <564171C2.8040700@ozlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <564171C2.8040700@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH v2 1/1] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: stewart@linux.vnet.ibm.com, benh@au1.ibm.com, nacc@linux.vnet.ibm.com, agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, paulus@au1.ibm.com, david@gibson.dropbear.id.au Alexey Kardashevskiy [aik@ozlabs.ru] wrote: | >| When exactly does a socket become a module? The SPAPR spec uses "sockets" here. | > | >I am trying to get the terminology too :-) Is socket a slot where a | >module is attached? | | Sorry, no idea. Ok. | | | > | >I will change the variable name 'modules' to 'sockets'. | >| | >| | >| >+ modinfo->si[0].chips = chips; | >| >+ modinfo->si[0].cores_per_chip = cores / chips; | >| | >| | >| What if no "ibm,chip-id" was found and chips == 0? | > | >If we fail to readdir(xscom) or fail to read the 'ibm,chip-id', | >we return an error which we check above. | | | You assume that if there is /proc/device-tree, then there is always | "xscom@" but this might not be always the case, like PR KVM on | embedded PPC64. For ibm,chip-id, we do try to read the file (and eliminate duplicates chip ids) If we can't read the file (hash_table_add_contents()) we return an error, but will check again. | | | | > | >| | >| | >| >+ | >| >+ return 0; | >| >+} | >| >diff --git a/hw/ppc/spapr_rtas_modinfo.h b/hw/ppc/spapr_rtas_modinfo.h | >| >new file mode 100644 | >| >index 0000000..356a2b5 | >| >--- /dev/null | >| >+++ b/hw/ppc/spapr_rtas_modinfo.h | >| | >| | >| This file is missing a license and | >| #ifndef SPAPR_RTAS_MODINFO_H | >| #define SPAPR_RTAS_MODINFO_H | >| #endif | >| | >| But I'd rather put the content to include/hw/ppc/spapr.h and avoid | >| creating new files. | > | >Ok. | > | >| | >| | >| >@@ -0,0 +1,12 @@ | >| >+ | >| >+struct rtas_socket_info { | >| >+ unsigned short sockets; | >| >+ unsigned short chips; | >| >+ unsigned short cores_per_chip; | >| >+}; | >| >+struct rtas_module_info { | >| >+ unsigned short module_types; | >| >+ struct rtas_socket_info si[1]; | >| >+}; | >| | >| | >| Will the number of rtas_socket_info be always a constant or (sure, | >| in the future) it may take different values? | > | >TBH, I am not sure. One thing though, array size of rtas_socket_info | >depends on number of module _types_. Like Nishanth Aravamudan mentioned | >offline, we are not sure if both a DCM and SCM types can be on a system | >at once (or even if they can be added dynamically). | | What are DCM and SCM? What type do we have in Tuleta? Dual Chip Module and Single Chip Module. Maybe its standard configuration but on our Tuleta, we have a DCM (2 chips per module): $ lsprop /proc/device-tree/xscom*/ibm,hw-module-id /proc/device-tree/xscom@3fc0000000000/ibm,hw-module-id 00000000 /proc/device-tree/xscom@3fc0800000000/ibm,hw-module-id 00000000 /proc/device-tree/xscom@3fc8000000000/ibm,hw-module-id 00000001 /proc/device-tree/xscom@3fc8800000000/ibm,hw-module-id 00000001 Each xscom represents a chip. Two chips have module id 0, two have module id 1. | | | >Another thing I have on my list to check is how this works with | >hotplug CPU (in the host). If the device-tree is properly updated | >then these calls will return the updated info? The values in the | >array will change but the number of entries (types) in the array is | >still 1. | > | >| | >| Normally when you invent API like rtas_get_module_info(), you tell | >| the helper how much memory is allocated in the rtas_module_info | >| struct (in our case it is ARRAY_SIZE(rtas_module_info.si) which is | >| fine) and then the helper signals somehow how many module types it | >| has found (which is missing here). | > | >Can we assume that the number of types is static for now i.e both | >caller and callee know the size (although I should fix the computation | >of size in spapr_rtas.c and use MAX_MODULE_TYPES that David pointed out). | | | I do not know what these modules can be so I cannot advise on this, | sorry. I honestly do not see much point in implementing this system | parameter - what will the guest do with this information? AFAICT, this system parameter is needed for distro licensing on powerKVM :-) | | You could get rid of rtas_module_info for now and have a helper for | rtas_socket_info only - rtas_get_xxx_module_info() (where xxx is DCM | or SCM or whatever it is now and what we support). Then it would be | up to rtas_ibm_get_system_parameter() to decide how many modules | types we want to expose to the guest (now - one) and when we get a | new module type, we will either extend rtas_get_xxx_module_info() or | implement another helper. | | | -- | Alexey