From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] KVM: Sanitize cpuid Date: Wed, 18 May 2011 11:34:44 +0300 Message-ID: <4DD384A4.7030701@redhat.com> References: <1305206625-28982-1-git-send-email-avi@redhat.com> <20110518071910.GJ31309@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , "kvm@vger.kernel.org" To: "Roedel, Joerg" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:51161 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755951Ab1ERIet (ORCPT ); Wed, 18 May 2011 04:34:49 -0400 In-Reply-To: <20110518071910.GJ31309@amd.com> Sender: kvm-owner@vger.kernel.org List-ID: On 05/18/2011 10:19 AM, Roedel, Joerg wrote: > > break; > > + case 0x80000008: { > > + u8 g_phys_as = entry->eax>> 16; > > + u8 virt_as = max(entry->eax>> 8, 48U); > > Shouldn't that be 'max((entry->eax>> 8)& 0xff, 48U)'? Seems safer when > the entry->eax contains a non-zero g_phys value. Yes, this is a bug. I originally had 'u8 virt_as = entry->eax >> 8', relying on the cast to u8, but missed it when updating. Moral: don't be subtle. > > + u8 phys_as = entry->eax; > > + > > + if (!g_phys_as) > > + g_phys_as = phys_as; > > + entry->eax = g_phys_as | (virt_as<< 8); > > It is optional, but since we support nesting we can also encode > g_phys_as in bits 23:16. I'm relying on a zero value in 23:16 indicating you should use 7:0. Thanks for the review, will post an updated patch. -- error compiling committee.c: too many arguments to function