[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/8] exec: Drop incorrect & dead S390 code in qe
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 5/8] exec: Drop incorrect & dead S390 code in qemu_ram_remap() |
Date: |
Wed, 19 Jun 2013 15:22:19 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Paolo Bonzini <address@hidden> writes:
> Il 19/06/2013 13:44, Markus Armbruster ha scritto:
>> Old S390 KVM wants guest RAM mapped in a peculiar way. Commit 6b02494
>> implemented that.
>>
>> When qemu_ram_remap() got added in commit cd19cfa, its code carefully
>> mimicked the allocation code: peculiar way if defined(TARGET_S390X) &&
>> defined(CONFIG_KVM), else normal way.
>>
>> For new S390 KVM, we actually want the normal way. Commit fdec991
>> changed qemu_ram_alloc_from_ptr() accordingly, but forgot to update
>> qemu_ram_remap(). If qemu_ram_alloc_from_ptr() maps RAM the normal
>> way, but qemu_ram_remap() remaps it the peculiar way, remapping
>> changes protection and flags, which it shouldn't.
>>
>> Fortunately, this can't happen, as we never remap on S390.
>>
>> Replace the incorrect code with an assertion.
>>
>> Thanks to Christian Borntraeger for help with assessing the bug's
>> (non-)impact.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> exec.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index c45eb33..a0f18fe 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1229,15 +1229,16 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t
>> length)
>> area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
>> flags, block->fd, offset);
>> } else {
>> -#if defined(TARGET_S390X) && defined(CONFIG_KVM)
>> - flags |= MAP_SHARED | MAP_ANONYMOUS;
>> - area = mmap(vaddr, length,
>> PROT_EXEC|PROT_READ|PROT_WRITE,
>> - flags, -1, 0);
>> -#else
>> + /*
>> + * Remap needs to match alloc. Accelerators that
>> + * set phys_mem_alloc never remap. If they did,
>> + * we'd need a remap hook here.
>> + */
>> + assert(!phys_mem_alloc);
>
> Probably "assert(phys_mem_alloc == qemu_anon_ram_alloc)"?
Of course. Will fix.
> Otherwise all looks fine.
Thanks!
- [Qemu-devel] [PATCH 1/8] exec: Fix Xen RAM allocation with unusual options, (continued)
- [Qemu-devel] [PATCH 1/8] exec: Fix Xen RAM allocation with unusual options, Markus Armbruster, 2013/06/19
- [Qemu-devel] [PATCH 2/8] exec: Clean up fall back when -mem-path allocation fails, Markus Armbruster, 2013/06/19
- [Qemu-devel] [PATCH 3/8] exec: Reduce ifdeffery around -mem-path, Markus Armbruster, 2013/06/19
- [Qemu-devel] [PATCH 4/8] exec: Simplify the guest physical memory allocation hook, Markus Armbruster, 2013/06/19
- [Qemu-devel] [PATCH 6/8] exec: Clean up unnecessary S390 ifdeffery, Markus Armbruster, 2013/06/19
- [Qemu-devel] [PATCH 8/8] pc_sysfw: Fix ISA BIOS init for ridiculously big flash, Markus Armbruster, 2013/06/19
- [Qemu-devel] [PATCH 5/8] exec: Drop incorrect & dead S390 code in qemu_ram_remap(), Markus Armbruster, 2013/06/19
- [Qemu-devel] [PATCH 7/8] exec: Don't abort when we can't allocate guest memory, Markus Armbruster, 2013/06/19