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: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: RE: [Xen-devel] hvmemul_virtual_to_linear() doesn't care about direction-flag?
From: "Byrne, John (HP Labs)" <john.l.byrne@xxxxxx>
Date: Tue, 19 Aug 2008 18:33:43 +0000
Accept-language: en-US
Acceptlanguage: en-US
Delivery-date: Tue, 19 Aug 2008 11:35:07 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C4D0C8A6.1C40E%keir.fraser@xxxxxxxxxxxxx>
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>
References: <469F2699A483D44BA6D2B311B1089D3A4338FC6019@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <C4D0C8A6.1C40E%keir.fraser@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AckBYgqYPA8ffyuBQxqCY+AgRFffRQAclVJ+AAUtT5kACPVKTQADeijQAAKjNooAAE65MA==
Thread-topic: [Xen-devel] hvmemul_virtual_to_linear() doesn't care about direction-flag?
You're welcome.

I was talking about a different ASSERT, see below, but I think I've answered my 
own concern. Given that, I don't see any problems.


> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx]
> Sent: Tuesday, August 19, 2008 11:00 AM
> To: Byrne, John (HP Labs); xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] hvmemul_virtual_to_linear() doesn't care about
> direction-flag?
> 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.

Wrong assert. I confused you, my fault. The ASSERT(!reverse) I figured out.

The ASSERT I was concerned about is this:

    if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1) )
        ASSERT(offset >= ((*reps - 1) * bytes_per_rep));

However, I just read truncate_ea_and_reps() in the emulator which should 
guarantee this.

> > 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.

Clear now, thanks. I wasn't reading it properly for some reason.

>  -- Keir

Xen-devel mailing list