Re: [Qemu-devel] [PATCH] isapc: Shadow ISA BIOS by default

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-10-12 15:41, Anthony Liguori wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> On 2012-10-08 20:52, Anthony Liguori wrote:
>>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>>
>>>> On 2012-09-11 17:53, Jan Kiszka wrote:
>>>>> Our one and only BIOS depends on a writable shadowed BIOS in the ISA
>>>>> range. As we have no interface to control the write property, make that
>>>>> region writable by default.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> ---
>>>>>
>>>>> This unbreaks isapc for TCG, and keep it working for KVM once it starts
>>>>> supporting read-only memslots.
>>>>>
>>>>>  hw/pc_sysfw.c |   13 +++++++++----
>>>>>  1 files changed, 9 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
>>>>> index b45f0ac..027d98a 100644
>>>>> --- a/hw/pc_sysfw.c
>>>>> +++ b/hw/pc_sysfw.c
>>>>> @@ -136,6 +136,7 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory)
>>>>>  {
>>>>>      char *filename;
>>>>>      MemoryRegion *bios, *isa_bios;
>>>>> +    void *isa_bios_ptr;
>>>>>      int bios_size, isa_bios_size;
>>>>>      int ret;
>>>>>  
>>>>> @@ -167,19 +168,23 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory)
>>>>>          g_free(filename);
>>>>>      }
>>>>>  
>>>>> -    /* map the last 128KB of the BIOS in ISA space */
>>>>> +    /* Shadow the last 128KB of the BIOS in ISA space as RAM -
>>>>> +     * Seabios depends on this */
>>>>>      isa_bios_size = bios_size;
>>>>>      if (isa_bios_size > (128 * 1024)) {
>>>>>          isa_bios_size = 128 * 1024;
>>>>>      }
>>>>>      isa_bios = g_malloc(sizeof(*isa_bios));
>>>>> -    memory_region_init_alias(isa_bios, "isa-bios", bios,
>>>>> -                             bios_size - isa_bios_size, isa_bios_size);
>>>>> +    memory_region_init_ram(isa_bios, "isa-bios", isa_bios_size);
>>>>> +    vmstate_register_ram_global(isa_bios);
>>>>>      memory_region_add_subregion_overlap(rom_memory,
>>>>>                                          0x100000 - isa_bios_size,
>>>>>                                          isa_bios,
>>>>>                                          1);
>>>>> -    memory_region_set_readonly(isa_bios, true);
>>>>> +
>>>>> +    /* copy ISA rom image from top of the ROM */
>>>>> +    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
>>>>> +    rom_copy(isa_bios_ptr, (uint32_t)(-isa_bios_size), isa_bios_size);
>>>>>  
>>>>>      /* map all the bios at the top of memory */
>>>>>      memory_region_add_subregion(rom_memory,
>>>>>
>>>>
>>>> Ping. Or already queued?
>>>
>>> I've got it queued now.  Thanks.
>>
>> I'm withdrawing it: This breaks Windows 95 booting. For unknown reasons,
>> this nice OS decided to overwrite the F-segment during boot. That is
>> fine as long as it is properly protected. But it breaks under current
>> KVM and with the patch above for the isapc. So we need a firmware
>> interface to enable/disable write protection for this segment in isapc
>> mode, specifically as that machine targets these old OSes.
> 
> Ah, if it wasn't for a build break caused by one of the pull requests, I
> would have pushed last night.  Thanks for the heads up, I'll remove it
> from my queue.
> 
> Is fw_cfg the right interface?  I presume this is i440fx specific?  How
> does q35 handle this?

No, there is no i440fx or q35 in that case. There are discrete chips
and wiring on an undefined ISA motherboard. As Seabios depends on a
writable E&F-segments (maybe only on E, still need to find out) for a
certain period, we need to invent a pv channel (probably via fw_cfg) to
provide the necessary control knob.

>  Presumably there's a second window for the BIOS
> mapping.  There's got to be some way to do shadowing of it I would
> think.

Not sure what you mean here. This is only about shadowing the top 128K
of the BIOS into the E/F-segment and providing a write-enable knob for it.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux