From mboxrd@z Thu Jan 1 00:00:00 1970 From: "H. Peter Anvin" Subject: Re: New CPUID/MSR driver; virtualization hooks Date: Thu, 05 Apr 2007 14:09:51 -0700 Message-ID: <4615659F.8070406@zytor.com> References: <461447F2.9010807@zytor.com> <20070405011640.GL19575@sequoia.sous-sol.org> <46144FA6.1000802@zytor.com> <4614867C.4060506@vmware.com> <461539DF.6010502@zytor.com> <46156591.8080802@vmware.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <46156591.8080802@vmware.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Zachary Amsden Cc: Chris Wright , Virtualization Mailing List List-Id: virtualization@lists.linuxfoundation.org Zachary Amsden wrote: > H. Peter Anvin wrote: >> >> This code is almost entirely identical to the setgpr_wrapper in the = >> patch (except for the fact that setgpr_wrapper sets and captures *ALL* = >> the GPRs), and it seems rather pointless to use another wrapper. It = >> takes a pointer to an entrypoint (default to "cpuid; ret" in the CPUID = >> case), so it should do what you need. > = > void (*cpuid)(unsigned int *eax, unsigned int *ebx, ...) > = > Not quite. The paravirt_ops CPUID function is a C function which takes = > pointers to each GPR as arguments, and returns the values through those = > pointers. This doesn't allow for an entrypoint compatible with = > setgpr_wrapper, which expects the same output in both the cpuid; ret = > case and the paravirt-ops case. > = > One could argue that the paravirt-ops CPUID should in fact emulate the = > native instruction semantics, which would make it a non-C function. = > However, until that bridge is crossed, we would need another wrapper = > after setgpr to convert the paravirt-ops CPUID back into the same format = > as native so that setgpr_wrapper can properly store the output fields = > into the common i386/x86_64 structure. What definitely should be done = > is hide this secondary wrapper in the paravirt-ops code so that your = > setgpr wrapper doesn't have to deal with weird issues like this because = > of paravirt-ops changes. I guess what I was trying to say was that we'd use setgpr_wrapper in the = case where you have an entrypoint with native (non-C) semantics; in the = other case we'd use an alternative to setgpr_wrapper. Either way, it = sounds like we're talking about implementing paravirtualization *after* = CPU selection, i.e. we use IPI to get the thing running on the proper = CPU before invoking the paravirtualization function? -hpa