From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH v3 1/4] xen/libxc: Allow changes to
 hypervisor CPUID leaf from config file
Date: Fri, 14 Mar 2014 11:41:15 -0400
Message-ID: <5323231B.9000607@oracle.com>
References: <1394734109-3192-1-git-send-email-boris.ostrovsky@oracle.com>
	<1394734109-3192-2-git-send-email-boris.ostrovsky@oracle.com>
	<5322C7510200007800124111@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <5322C7510200007800124111@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, eddie.dong@intel.com, xen-devel@lists.xen.org, jun.nakajima@intel.com, ian.jackson@eu.citrix.com
List-Id: xen-devel@lists.xenproject.org

On 03/14/2014 04:09 AM, Jan Beulich wrote:
>>>> On 13.03.14 at 19:08, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote:
>> @@ -43,22 +45,29 @@ static int hypervisor_is_64bit(xc_interface *xch)
>>   static void cpuid(const unsigned int *input, unsigned int *regs)
>>   {
>>       unsigned int count = (input[1] == XEN_CPUID_INPUT_UNUSED) ? 0 : input[1];
>> +    uint8_t is_hyp = IS_HYPERVISOR_LEAF(input[0]);
>>   #ifdef __i386__
>>       /* Use the stack to avoid reg constraint failures with some gcc flags */
>>       asm (
>>           "push %%ebx; push %%edx\n\t"
>> -        "cpuid\n\t"
>> +        "testb $0xff,%5\n\t"
>> +        "jz 1f\n\t"
>> +        XEN_EMULATE_PREFIX
>> +        "1: cpuid\n\t"
>>           "mov %%ebx,4(%4)\n\t"
>>           "mov %%edx,12(%4)\n\t"
>>           "pop %%edx; pop %%ebx\n\t"
>>           : "=a" (regs[0]), "=c" (regs[2])
>> -        : "0" (input[0]), "1" (count), "S" (regs)
>> +        : "0" (input[0]), "1" (count), "S" (regs), "m" (is_hyp)
> All inputs must be in registers here, since memory references might
> use %esp and hence be off by 2 stack slots due to the pushes/pops
> surrounding the actual operation. Since you evaluate the flag prior
> to the CPUID, using "db" as constraint would seem possible here.

gcc for some reason rejects "b" as inconsistent (but "d" works fine).


>> @@ -555,6 +564,15 @@ static int xc_cpuid_policy(
>>   {
>>       xc_dominfo_t        info;
>>   
>> +    if ( IS_HYPERVISOR_LEAF(input[0]) )
>> +    {
>> +        /* Only leaf 1 can be modified */
>> +        if ( input[0] == 0x40000000 )
>> +            return 0;
>> +        else
>> +            return -EACCES;
> And I'm still worried about altering this in uncontrolled ways: No
> good can result from allowing ecx/edx/ebx to be modified, and
> improperly modifying the eax value (namely putting in place
> wrong upper bits, the more that those aren't statically
> determined) won't help much either. IOW it should really only
> be the low 8 bits of eax that can be overridden, and it shouldn't
> be possible to clear these 8 bits.

I'll add mask array to xc_cpuid_policy (and probably other policy
routines that it calls) that will specify bits that are permitted to be
overwritten.

-boris