From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57813) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6ZmK-0001cO-HH for qemu-devel@nongnu.org; Wed, 21 Sep 2011 23:16:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R6ZmI-0003Pv-TN for qemu-devel@nongnu.org; Wed, 21 Sep 2011 23:16:48 -0400 Received: from ozlabs.org ([203.10.76.45]:36748) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6ZmI-0003Pp-63 for qemu-devel@nongnu.org; Wed, 21 Sep 2011 23:16:46 -0400 Date: Thu, 22 Sep 2011 13:16:40 +1000 From: David Gibson Message-ID: <20110922031640.GL22223@yookeroo.fritz.box> References: <1316586940-5692-1-git-send-email-david@gibson.dropbear.id.au> <3AB79E4E-4C73-4699-9EF9-59105F72FB12@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3AB79E4E-4C73-4699-9EF9-59105F72FB12@suse.de> Subject: Re: [Qemu-devel] [PATCH] pseries: Add device tree properties for VMX/VSX and DFP under kvm List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: qemu-devel@nongnu.org On Wed, Sep 21, 2011 at 09:53:22AM +0200, Alexander Graf wrote: > > On 21.09.2011, at 08:35, David Gibson wrote: > > > Sufficiently recent PAPR specifications define properties "ibm,vmx" > > and "ibm,dfp" on the CPU node which advertise whether the VMX vector > > extensions (or the later VSX version) and/or the Decimal Floating > > Point operations from IBM's recent POWER CPUs are available. > > > > Currently we do not put these in the guest device tree and the guest > > kernel will consequently assume they are not available. This is good, > > because they are not supported under TCG. VMX is similar enough to > > Altivec that it might be trivial to support, but VSX and DFP would > > both require significant work to support in TCG. > > > > However, when running under kvm on a host which supports these > > instructions, there's no reason not to let the guest use them. This > > patch, therefore, checks for the relevant support on the host CPU > > and, if present, advertises them to the guest as well. > > > > Signed-off-by: David Gibson > > --- > > hw/spapr.c | 13 ++++++++++ > > target-ppc/kvm.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > target-ppc/kvm_ppc.h | 12 ++++++++++ > > 3 files changed, 85 insertions(+), 0 deletions(-) > > > > diff --git a/hw/spapr.c b/hw/spapr.c > > index b118975..573392d 100644 > > --- a/hw/spapr.c > > +++ b/hw/spapr.c > > @@ -168,6 +168,9 @@ static void *spapr_create_fdt_skel(const char *cpu_model, > > 0xffffffff, 0xffffffff}; > > uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ; > > uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000; > > + /* Currently TCG doesn't implement VMX or DFP instructions */ > > + uint32_t vmx = kvm_enabled() ? kvmppc_get_vmx() : 0; > > + uint32_t dfp = kvm_enabled() ? kvmppc_get_dfp() : 0; > > This should also take the target CPU into account. If the target CPU > can't do VMX/DFP, it shouldn' be announced. Imagine running a guest > with the compatibility mode bit set, so we're virtualization the > previous generation. Hrm, ok. > When did dfp get introduced? Was that POWER6 or POWER7? POWER7, I think, but I'm not certain. > Also, isn't > there this twice-as-wide VMX extension too? Yeah, that's VSX. It's represented by a 2 instead of a 1 in the "ibm,vmx" property. > > if (asprintf(&nodename, "%s@%x", modelname, index) < 0) { > > fprintf(stderr, "Allocation failure\n"); > > @@ -202,6 +205,16 @@ static void *spapr_create_fdt_skel(const char *cpu_model, > > segs, sizeof(segs)))); > > } > > > > + /* Advertise VMX/VSX (vector extensions) if available */ > > + if (vmx) { > > + _FDT((fdt_property_cell(fdt, "ibm,vmx", vmx))); > > + } > > + > > + /* Advertise DFP (Decimal Floating Point) if available */ > > + if (dfp) { > > + _FDT((fdt_property_cell(fdt, "ibm,dfp", dfp))); > > + } > > + > > _FDT((fdt_end_node(fdt))); > > } > > > > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > > index 35a6f10..397a803 100644 > > --- a/target-ppc/kvm.c > > +++ b/target-ppc/kvm.c > > @@ -673,6 +673,66 @@ uint64_t kvmppc_get_clockfreq(void) > > return 0; > > } > > > > +uint32_t kvmppc_get_vmx(void) > > +{ > > + char buf[512]; > > + uint32_t vmx; > > + FILE *f; > > + int len; > > + > > + if (kvmppc_find_cpu_dt(buf, sizeof(buf))) { > > + return 0; > > + } > > + > > + strncat(buf, "/ibm,vmx", sizeof(buf) - strlen(buf)); > > + > > + f = fopen(buf, "rb"); > > + if (!f) { > > + /* Assume -ENOENT, which indicates that VMX is not available */ > > + return 0; > > + } > > + > > + len = fread(&vmx, sizeof(vmx), 1, f); > > + fclose(f); > > + > > + if (len != 1) { > > + /* Malformed ibm,vmx property, assume no vmx or vsx */ > > + return 0; > > + } > > + > > + return be32_to_cpu(vmx); > > +} > > + > > +uint32_t kvmppc_get_dfp(void) > > +{ > > + char buf[512]; > > + uint32_t dfp; > > + FILE *f; > > + int len; > > + > > + if (kvmppc_find_cpu_dt(buf, sizeof(buf))) { > > + return 0; > > + } > > + > > + strncat(buf, "/ibm,dfp", sizeof(buf) - strlen(buf)); > > + > > + f = fopen(buf, "rb"); > > + if (!f) { > > + /* Assume -ENOENT, which indicates that DFP is not available */ > > + return 0; > > + } > > + > > + len = fread(&dfp, sizeof(dfp), 1, f); > > + fclose(f); > > + > > + if (len != 1) { > > + /* Malformed ibm,dfp property, assume no DFP */ > > + return 0; > > + } > > + > > + return be32_to_cpu(dfp); > > +} > > Could you please fold those two into a single helper function to > read out a 32-bit dt property and then just use that? :) > > Please also document somewhere in the code what the return value > means (0 = unavailable, 1 = available). Ok. -- 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