From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1L2Xyf-0001E4-NA
	for qemu-devel@nongnu.org; Tue, 18 Nov 2008 16:19:17 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1L2Xyf-0001Dr-Ai
	for qemu-devel@nongnu.org; Tue, 18 Nov 2008 16:19:17 -0500
Received: from [199.232.76.173] (port=52032 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1L2Xyf-0001Dn-4a
	for qemu-devel@nongnu.org; Tue, 18 Nov 2008 16:19:17 -0500
Received: from ey-out-1920.google.com ([74.125.78.144]:8058)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <anthony@codemonkey.ws>) id 1L2Xyf-0004nt-6h
	for qemu-devel@nongnu.org; Tue, 18 Nov 2008 16:19:17 -0500
Received: by ey-out-1920.google.com with SMTP id 4so1302771eyk.4
	for <qemu-devel@nongnu.org>; Tue, 18 Nov 2008 13:19:15 -0800 (PST)
Message-ID: <4923314F.2090604@codemonkey.ws>
Date: Tue, 18 Nov 2008 15:19:11 -0600
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH v5 17/18] gdbstub: x86: Support for setting
	segment registers
References: <20081117161857.26880.45423.stgit@mchn012c.ww002.siemens.net>
	<20081117161859.26880.26254.stgit@mchn012c.ww002.siemens.net>
In-Reply-To: <20081117161859.26880.26254.stgit@mchn012c.ww002.siemens.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: qemu-devel@nongnu.org

Jan Kiszka wrote:
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index eed1f62..b7c8a2f 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -651,6 +651,20 @@ int cpu_get_pic_interrupt(CPUX86State *s);
>  /* MSDOS compatibility mode FPU exception support */
>  void cpu_set_ferr(CPUX86State *s);
>  
> +static inline unsigned int get_seg_limit(uint32_t e1, uint32_t e2)
> +{
> +    unsigned int limit;
> +    limit = (e1 & 0xffff) | (e2 & 0x000f0000);
> +    if (e2 & DESC_G_MASK)
> +        limit = (limit << 12) | 0xfff;
> +    return limit;
> +}
> +
> +static inline uint32_t get_seg_base(uint32_t e1, uint32_t e2)
> +{
> +    return ((e1 >> 16) | ((e2 & 0xff) << 16) | (e2 & 0xff000000));
> +}
> +
>   

I like this patch but if you're going to export new x86 helper 
functions, please prefix them with cpu_x86. In this case, these helpers 
are awfully low level (they're taking a GDT entry split into two 32-bit 
words).  I would rather see an interface that took a CPUState and a 
segment register index.  In the very least, it won't be very obvious to 
anyone what this API expects to take so a comment would be required.

Regards,

Anthony Liguori