Il 31/07/2014 14:01, Peter Crosthwaite ha scritto:
> On Wed, Jul 30, 2014 at 8:27 PM, Paolo Bonzini <pbonz...@redhat.com> wrote:
>> Instead, add a boolean variable to indicate the presence of the region.
>>
>
> Patch is good. Can you add a sentence on why you are doing this
> though? Is it just the save on the repeated malloc and free (which is
> sane in its own right) or something bigger in the context of your
> series?
Even more than the repeated malloc and free, it's the repeated
add_child/unparent.
Paolo
>> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>
> Otherwise,
>
> Reviewed-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
>
>> ---
>> hw/display/vga.c | 24 ++++++++++--------------
>> hw/display/vga_int.h | 3 ++-
>> 2 files changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/display/vga.c b/hw/display/vga.c
>> index 4b089a3..68cfee2 100644
>> --- a/hw/display/vga.c
>> +++ b/hw/display/vga.c
>> @@ -168,15 +168,18 @@ static uint8_t expand4to8[16];
>>
>> static void vga_update_memory_access(VGACommonState *s)
>> {
>> - MemoryRegion *region, *old_region = s->chain4_alias;
>> hwaddr base, offset, size;
>>
>> if (s->legacy_address_space == NULL) {
>> return;
>> }
>>
>> - s->chain4_alias = NULL;
>> -
>> + if (s->has_chain4_alias) {
>> + memory_region_del_subregion(s->legacy_address_space,
>> &s->chain4_alias);
>> + memory_region_destroy(&s->chain4_alias);
>> + s->has_chain4_alias = false;
>> + s->plane_updated = 0xf;
>> + }
>> if ((s->sr[VGA_SEQ_PLANE_WRITE] & VGA_SR02_ALL_PLANES) ==
>> VGA_SR02_ALL_PLANES && s->sr[VGA_SEQ_MEMORY_MODE] &
>> VGA_SR04_CHN_4M) {
>> offset = 0;
>> @@ -201,18 +204,11 @@ static void vga_update_memory_access(VGACommonState *s)
>> break;
>> }
>> base += isa_mem_base;
>> - region = g_malloc(sizeof(*region));
>> - memory_region_init_alias(region, memory_region_owner(&s->vram),
>> + memory_region_init_alias(&s->chain4_alias,
>> memory_region_owner(&s->vram),
>> "vga.chain4", &s->vram, offset, size);
>> memory_region_add_subregion_overlap(s->legacy_address_space, base,
>> - region, 2);
>> - s->chain4_alias = region;
>> - }
>> - if (old_region) {
>> - memory_region_del_subregion(s->legacy_address_space, old_region);
>> - memory_region_destroy(old_region);
>> - g_free(old_region);
>> - s->plane_updated = 0xf;
>> + &s->chain4_alias, 2);
>> + s->has_chain4_alias = true;
>> }
>> }
>>
>> @@ -1321,7 +1317,7 @@ static void vga_draw_text(VGACommonState *s, int
>> full_update)
>> s->font_offsets[1] = offset;
>> full_update = 1;
>> }
>> - if (s->plane_updated & (1 << 2) || s->chain4_alias) {
>> + if (s->plane_updated & (1 << 2) || s->has_chain4_alias) {
>> /* if the plane 2 was modified since the last display, it
>> indicates the font may have been modified */
>> s->plane_updated = 0;
>> diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
>> index 5320abd..641f8f4 100644
>> --- a/hw/display/vga_int.h
>> +++ b/hw/display/vga_int.h
>> @@ -94,7 +94,8 @@ typedef struct VGACommonState {
>> uint32_t vram_size;
>> uint32_t vram_size_mb; /* property */
>> uint32_t latch;
>> - MemoryRegion *chain4_alias;
>> + bool has_chain4_alias;
>> + MemoryRegion chain4_alias;
>> uint8_t sr_index;
>> uint8_t sr[256];
>> uint8_t gr_index;
>> --
>> 1.8.3.1
>>
>>
>>
