This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] hvmemul_virtual_to_linear() doesn't care about direction

To: "Byrne, John (HP Labs)" <john.l.byrne@xxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] hvmemul_virtual_to_linear() doesn't care about direction-flag?
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Tue, 19 Aug 2008 18:59:50 +0100
Delivery-date: Tue, 19 Aug 2008 11:00:16 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <469F2699A483D44BA6D2B311B1089D3A4338FC6019@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AckBYgqYPA8ffyuBQxqCY+AgRFffRQAclVJ+AAUtT5kACPVKTQADeijQAAKjNoo=
Thread-topic: [Xen-devel] hvmemul_virtual_to_linear() doesn't care about direction-flag?
User-agent: Microsoft-Entourage/
On 19/8/08 18:04, "Byrne, John (HP Labs)" <john.l.byrne@xxxxxx> wrote:

Thanks for taking a look!

> Major:
> 1.) In hvmemul_virtual_to_linear(), you've added the min() (line 299) on reps
> for reasons I don't understand and the ASSERT (line 304) in the reverse case.
> I don't see anything, anywhere, that guarantee that the ASSERT is true...and
> it needs to be for the code to be correct. If the min() is meant to guarantee
> this somehow, I don't see how it does. If it isn't meant to do this, I don't
> understand what it is for, as written.

The min() is to avoid overflow when multiplying by bytes_per_rep. It's
obviously a very conservative clip, but it's the same maximum we use in
hvmemul_linear_to_phys() so there's no point using a larger value. Changeset
18342 now adds a comment to this effect.

The assertion is subtle. Notice that on entry to the for-loop when
reverse=true then it is always the case that done >= bytes_per_rep. Hence
done/bytes_per_rep != 0 and the assertion must hold.

> Minor:
> 2.) In hvmemul_linear_to_phys(), you changed the exception injection (line
> 265) to use (addr & PAGE_MASK) instead of addr? Seems wrong, but there could
> easily be something I don't understand.

In the forward (EFLAGS.DF=0) case, if we span multiple pages then a
page-fault on any other than the first page in the range will always have
the faulting linear address at the first byte of the page. The first page is
of course a special case but that's dealt with separately on line 240. I
changed to addr&PAGE_MASK because I changed how addr is updated in the loop.
Notice I no longer add done on the first iteration but now always add
PAGE_SIZE. Previously it was guaranteed that addr would be page-aligned --
now I have to force it when injecting the exception.

 -- Keir

Xen-devel mailing list