Le 20/03/2018 à 19:49, Luke Shumaker a écrit : > On Fri, 02 Mar 2018 09:13:12 -0500, > Peter Maydell wrote: >> On 28 December 2017 at 18:08, Luke Shumaker <luke...@lukeshu.com> wrote: >>> + guest_full_size = >>> + (0xffff0f00 & qemu_host_page_mask) + qemu_host_page_size; > ^ >> I think this is probably more clearly written as 0x100000000ULL, >> since rounding down to the host-page-size then adding the host-page-size >> gets us the full 32-bit size of the guest address space. > > Wait, is that right? Isn't that only true if qemu_host_page_size is > at least 8KiB (16 bits), enough to fill the zero in the middle? Won't > a typical qemu_host_page_size be only 4KiB? > >> That shows up that there's a potential problem here if the host >> is 32-bit, because in that case guest_full_size (being only unsigned >> long) will be 0, and we'll end up trying an mmap with an incorrect size. >> >>> + host_full_size = guest_full_size - guest_start; >>> + real_start = (unsigned long) >>> + mmap(NULL, host_full_size, PROT_NONE, flags, -1, 0); >> >> I think the general approach is right, though. Sorry it took so long >> for us to get to reviewing this patchset. > > It's all good. I'm amazed at the amount of traffic qemu-devel gets! > >> Incidentally, this code would be rather less complicated if it didn't >> have to account for qemu_host_page_size not actually being the host >> page size (since then you couldn't get a return from mmap() that wasn't >> aligned properly). Does anybody know why we allow the user to specify >> it on the command line? (git revision history doesn't help, it just says >> there's been a -pagesize argument since commit 54936004fddc5 in 2003, >> right back when mmap emulation was first added...) > > I have no idea, I just assumed that it was a feature useful to people > far smarter than me. >
I'm going to add this patch in my upcoming linux-user pull-request (currently running regression tests). Thanks, Laurent
