Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-12 17:49, Jan Kiszka wrote:
> On 2011-09-12 17:45, Andreas Färber wrote:
>> Am 12.09.2011 17:33, schrieb Jan Kiszka:
>>> On 2011-09-12 17:20, Alexander Graf wrote:
>>>> Jan Kiszka wrote:
>>>>> Most VGA memory access modes require MMIO handling as they demand weird
>>>>> logic to get a byte from or into the video RAM. However, there is one
>>>>> exception: chain 4 mode with all memory planes enabled for writing. This
>>>>> mode actually allows lineary mapping, which can then be combined with
>>>>> dirty logging to accelerate KVM.
>>>>>
>>>>> This patch accelerates specifically VBE accesses like they are used by
>>>>> grub in graphical mode. Not only the standard VGA adapter benefits from
>>>>> this, also vmware and spice in VGA mode.
>>>>>
>>>>> CC: Gerd Hoffmann <address@hidden>
>>>>> CC: Avi Kivity <address@hidden>
>>>>> Signed-off-by: Jan Kiszka <address@hidden>
>>>>>   
>>>> [...]
>>>>
>>>>> +static void vga_update_memory_access(VGACommonState *s)
>>>>> +{
>>>>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>>>>> +    target_phys_addr_t base, offset, size;
>>>>> +
>>>>> +    s->chain4_alias = NULL;
>>>>> +
>>>>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>>>>> +        offset = 0;
>>>>> +        switch ((s->gr[6] >> 2) & 3) {
>>>>> +        case 0:
>>>>> +            base = 0xa0000;
>>>>> +            size = 0x20000;
>>>>> +            break;
>>>>> +        case 1:
>>>>> +            base = 0xa0000;
>>>>> +            size = 0x10000;
>>>>> +            offset = s->bank_offset;
>>>>> +            break;
>>>>> +        case 2:
>>>>> +            base = 0xb0000;
>>>>> +            size = 0x8000;
>>>>> +            break;
>>>>> +        case 3:
>>>>> +            base = 0xb8000;
>>>>> +            size = 0x8000;
>>>>> +            break;
>>>>> +        }
>>>>> +        region = g_malloc(sizeof(*region));
>>>>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, 
>>>>> size);
>>>>> +        memory_region_add_subregion_overlap(s->legacy_address_space, 
>>>>> base,
>>>>> +                                            region, 2);
>>>>>   
>>>> This one eventually gives me the following in info mtree with -M g3beige
>>>> on qemu-system-ppc:
>>>>
>>>> (qemu) info mtree
>>>> memory
>>>> system addr 00000000 off 00000000 size 7fffffffffffffff
>>>> -vga.chain4 addr 000a0000 off 00000000 size 10000
>>>> -macio addr 80880000 off 00000000 size 80000
>>>> --macio-nvram addr 00060000 off 00000000 size 20000
>>>> --pmac-ide addr 00020000 off 00000000 size 1000
>>>> --cuda addr 00016000 off 00000000 size 2000
>>>> --escc-bar addr 00013000 off 00000000 size 40
>>>> --dbdma addr 00008000 off 00000000 size 1000
>>>> --heathrow-pic addr 00000000 off 00000000 size 1000
>>>> -vga.rom addr 80800000 off 00000000 size 10000
>>>> -vga.vram addr 80000000 off 00000000 size 800000
>>>> -vga-lowmem addr 800a0000 off 00000000 size 20000
>>>> -escc addr 80013000 off 00000000 size 40
>>>> -isa-mmio addr fe000000 off 00000000 size 200000
>>>> I/O
>>>> io addr 00000000 off 00000000 size 10000
>>>> -cmd646-bmdma addr 00000700 off 00000000 size 10
>>>> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
>>>> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
>>>> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
>>>> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
>>>> -cmd646-cmd addr 00000680 off 00000000 size 4
>>>> -cmd646-data addr 00000600 off 00000000 size 8
>>>> -cmd646-cmd addr 00000580 off 00000000 size 4
>>>> -cmd646-data addr 00000500 off 00000000 size 8
>>>> -ne2000 addr 00000400 off 00000000 size 100
>>>>
>>>> This ends up overmapping 0xa0000, effectively overwriting kernel data.
>>>> If I #if 0 the offending chunk out, everything is fine. I would assume
>>>> that chain4 really needs to be inside of lowmem? No idea about VGA, but
>>>> I'm sure you know what's going on :).
>>> Does this help?
>>>
>>> diff --git a/hw/vga.c b/hw/vga.c
>>> index 125fb29..0a0c5a6 100644
>>> --- a/hw/vga.c
>>> +++ b/hw/vga.c
>>> @@ -181,6 +181,7 @@ static void vga_update_memory_access(VGACommonState *s)
>>>              size = 0x8000;
>>>              break;
>>>          }
>>> +        base += isa_mem_base;
>>>          region = g_malloc(sizeof(*region));
>>>          memory_region_init_alias(region, "vga.chain4", &s->vram, offset, 
>>> size);
>>>          memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>
>> No longer oopses, but the screen looks chaotic now (black bar at bottom,
>> part of contents at top etc.).
> 
> Does this PPC machine map the ISA range and forward VGA accesses to the
> adapter in general?

If it does, please post a dump of the VGACommonState while the screen is
corrupted (gdb or via device_show [1]. Maybe I missed some condition
that prevents chain4 optimizations, and your guest triggers this.

Jan

[1] http://thread.gmane.org/gmane.comp.emulators.qemu/114853

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux