From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:38359)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <avi@redhat.com>) id 1SYwLn-0002Tr-Vd
	for qemu-devel@nongnu.org; Mon, 28 May 2012 05:34:57 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <avi@redhat.com>) id 1SYwLk-0001gL-4H
	for qemu-devel@nongnu.org; Mon, 28 May 2012 05:34:55 -0400
Received: from mx1.redhat.com ([209.132.183.28]:53839)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <avi@redhat.com>) id 1SYwLj-0001gH-Sn
	for qemu-devel@nongnu.org; Mon, 28 May 2012 05:34:52 -0400
Message-ID: <4FC346B6.7060400@redhat.com>
Date: Mon, 28 May 2012 12:34:46 +0300
From: Avi Kivity <avi@redhat.com>
MIME-Version: 1.0
References: <4FBD9E3A.6080704@web.de>
	<CAMo8Bf+pWij2ojdGT02Fyo+DmbMjxvAzOm6sV_f-D7vaJDE_AQ@mail.gmail.com>
	<4FBE1A98.3090708@siemens.com>
	<CAMo8BfKN15QHRLPhtVc4mRF8bsKwZO=FiF0gU9SdL6qpY-LWWQ@mail.gmail.com>
	<4FBE26AF.2090103@siemens.com>
	<CAMo8BfK-9b5c=dNh1bA7NaXY-a9N1Yu6=OShTjMdLa1vyDMSZA@mail.gmail.com>
	<4FBE3716.6040205@siemens.com>
	<CAMo8BfJx6V2e4qKQOMXtTQE4W1JCrEMLKvxPqybE4LjQ-FZ+Ow@mail.gmail.com>
	<4FBE4518.4000402@redhat.com>
	<CAMo8Bf+u2DRR-LiUgzBorCjzwCeQPtN4cN-SENOi5zftXVAFfg@mail.gmail.com>
In-Reply-To: <CAMo8Bf+u2DRR-LiUgzBorCjzwCeQPtN4cN-SENOi5zftXVAFfg@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] TCG: Fix TB invalidation after breakpoint
	insertion/deletion
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Max Filippov <jcmvbkbc@gmail.com>
Cc: Blue Swirl <blauwirbel@gmail.com>, Jan Kiszka <jan.kiszka@siemens.com>, qemu-devel <qemu-devel@nongnu.org>, TeLeMan <geleman@gmail.com>

On 05/24/2012 10:58 PM, Max Filippov wrote:
> On Thu, May 24, 2012 at 6:26 PM, Avi Kivity <avi@redhat.com> wrote:
>> On 05/24/2012 05:11 PM, Max Filippov wrote:
>>>>
>>>> Not in breakpoint_invalidate as the missing offset was compensated
>>>> before your commit (well, starting with c2f07f81a2 in fact).
>>>
>>> I'd say that compensation that you mention
>>>
>>>     ram_addr = (memory_region_get_ram_addr(section.mr)
>>>                 + section.offset_within_region) & TARGET_PAGE_MASK;
>>> this >>>>    ram_addr |= (pc & ~TARGET_PAGE_MASK);
>>>     tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0);
>>>
>>> was removed by f3705d53296d, not by 1e7855a558
>>
>> Indeed.  Note how the |= cleverly accommodates both truncating and
>> non-truncating cpu_get_phys_page_debug().
> 
> Right. If the fix is going to be checked in then TeLeMan's original version
> with '|' is preferable for this reason.

I disagree.  Whatever we call cpu_get_phys_page_debug() has to either
mask out the low bits, or not (I prefer the latter, since it's
unambiguous for large pages), but it has to be consistent.  Once it's
consistent, there's no reason to use clever tricks.


-- 
error compiling committee.c: too many arguments to function