[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses base
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts |
Date: |
Tue, 16 Jul 2013 14:33:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 |
Il 16/07/2013 14:28, Jan Kiszka ha scritto:
> On 2013-05-30 23:03, Paolo Bonzini wrote:
>> This provides the basics for detecting accesses to unassigned memory
>> as soon as they happen, and also for a simple implementation of
>> address_space_access_valid.
>>
>> Reviewed-by: Richard Henderson <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>> exec.c | 36 ++++++++++++------------------------
>> memory.c | 28 ++++++++++++++++++++++++++--
>> 2 files changed, 38 insertions(+), 26 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 785eeeb..c5100d6 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
>> return ram_addr;
>> }
>>
>> -static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
>> - unsigned size)
>> +static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
>> + unsigned size, bool is_write)
>> {
>> -#ifdef DEBUG_UNASSIGNED
>> - printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> -#endif
>> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
>> -#endif
>> - return 0;
>> -}
>> -
>> -static void unassigned_mem_write(void *opaque, hwaddr addr,
>> - uint64_t val, unsigned size)
>> -{
>> -#ifdef DEBUG_UNASSIGNED
>> - printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr,
>> val);
>> -#endif
>> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
>> -#endif
>> + return false;
>> }
>>
>> -static const MemoryRegionOps unassigned_mem_ops = {
>> - .read = unassigned_mem_read,
>> - .write = unassigned_mem_write,
>> +const MemoryRegionOps unassigned_mem_ops = {
>> + .valid.accepts = unassigned_mem_accepts,
>> .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>>
>> @@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr
>> ram_addr,
>> tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
>> }
>>
>> +static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
>> + unsigned size, bool is_write)
>> +{
>> + return is_write;
>> +}
>> +
>> static const MemoryRegionOps notdirty_mem_ops = {
>> - .read = unassigned_mem_read,
>> .write = notdirty_mem_write,
>> + .valid.accepts = notdirty_mem_accepts,
>> .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>>
>> diff --git a/memory.c b/memory.c
>> index 99f046d..15da877 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
>> mr->flush_coalesced_mmio = false;
>> }
>>
>> +static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
>> + unsigned size)
>> +{
>> +#ifdef DEBUG_UNASSIGNED
>> + printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> +#endif
>> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
>> +#endif
>> + return 0;
>
> This changed the value read from unassigned memory from -1 to 0. Any
> particular reason or an unintentional change?
Cut-and-paste (unassigned RAM used to return 0, invalid MMIO used to
return -1, unifying the paths dropped the difference).
I guess unassigned RAM can read as -1 just fine, so we can just change
unassigned_mem_read to return -1.
Paolo
> Jan
>
>> +}
>> +
>> +static void unassigned_mem_write(void *opaque, hwaddr addr,
>> + uint64_t val, unsigned size)
>> +{
>> +#ifdef DEBUG_UNASSIGNED
>> + printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr,
>> val);
>> +#endif
>> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
>> +#endif
>> +}
>> +
>> static bool memory_region_access_valid(MemoryRegion *mr,
>> hwaddr addr,
>> unsigned size,
>> @@ -847,7 +870,7 @@ static uint64_t
>> memory_region_dispatch_read1(MemoryRegion *mr,
>> uint64_t data = 0;
>>
>> if (!memory_region_access_valid(mr, addr, size, false)) {
>> - return -1U; /* FIXME: better signalling */
>> + return unassigned_mem_read(mr, addr, size);
>> }
>>
>> if (!mr->ops->read) {
>> @@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion
>> *mr,
>> unsigned size)
>> {
>> if (!memory_region_access_valid(mr, addr, size, true)) {
>> - return; /* FIXME: better signalling */
>> + unassigned_mem_write(mr, addr, data, size);
>> + return;
>> }
>>
>> adjust_endianness(mr, &data, size);
>>