[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range. |
Date: |
Fri, 12 Feb 2010 22:37:06 +0200 |
On Fri, Feb 12, 2010 at 10:16 PM, Richard Henderson <address@hidden> wrote:
> On 02/12/2010 11:47 AM, Blue Swirl wrote:
>>
>> Please make separate patches for unrelated changes. Now the essence of
>> the patch is very hard to see. Also pure formatting changes are not
>> very useful.
>
> Is this just about page_get_flags, or was there some other pure formatting
> change to which you object? For instance:
Also this one:
- end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose
bits in the next step */
+ /* END must be computed before we drop bits from START. */
+ end = TARGET_PAGE_ALIGN(start + len);
start = start & TARGET_PAGE_MASK;
and these:
- if( !p )
+ if (!p)
return -1;
- if( !(p->flags & PAGE_VALID) )
+ if (!(p->flags & PAGE_VALID))
>
>>> - if (start + len < start)
>>> - /* we've wrapped around */
>>> + if (start + len - 1 < start) {
>>> + /* We've wrapped around. */
>>> return -1;
>>> + }
>
> Only the first line is required to fix an off-by-one error. But if I don't
> add the braces, generally the patch will be bounced for that.
That change is OK.
>>> - /* We may be called for host regions that are outside guest
>>> - address space. */
>>
>> Why remove the comment, is it no longer true?
>
> Yes, after the entire patch series is applied. Indeed, I believe that many
> of the addresses rejected here were *not* in fact outside the guest address
> space, merely that page_find_alloc did not accurately know what the guest
> address space was.
>
> I think it was a mistake to ever consider usage of this function with
> out-of-band addresses as valid -- it adds a great deal of confusion.
>
> I could add an assertion here to make sure, if you like.
>
> If this were C++, it might have been interesting to try to use the type
> system to keep the host and guest address spaces forcibly separate. But
> since this is plain C, I think wrapping everything in structures and access
> macros would just be too ugly.
>
>
> r~
>
- [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes., Richard Henderson, 2010/02/11
- [Qemu-devel] [PATCH 4/6] Implement multi-level page tables., Richard Henderson, 2010/02/11
- [Qemu-devel] [PATCH 2/6] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid., Richard Henderson, 2010/02/11
- [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h., Richard Henderson, 2010/02/11
- [Qemu-devel] [PATCH 6/6] linux-user: Fix mmap_find_vma returning invalid addresses., Richard Henderson, 2010/02/11
- [Qemu-devel] [PATCH 5/6] linux-user: Use h2g_valid in qemu_vmalloc., Richard Henderson, 2010/02/11
- [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2, Richard Henderson, 2010/02/15