Am 30. April 2024 15:39:21 UTC schrieb "Philippe Mathieu-Daudé" 
<phi...@linaro.org>:
>On 30/4/24 17:06, Bernhard Beschow wrote:
>> In the -bios case the "isa-bios" memory region is an alias to the BIOS mapped
>> to the top of the 4G memory boundary. Do the same in the -pflash case, but 
>> only
>> for new machine versions for migration compatibility. This establishes common
>> behavior and makes pflash commands work in the "isa-bios" region which some
>> real-world legacy bioses rely on.
>
>Can you amend a diff of 'info mtree' here to see how the layout changes?

Will do.

Right now I have to manually sort the output to get a minimal diff. Is there a 
way to get a stable ordering of the memory regions? How would one fix that if 
this is currently impossible? With stable orderings we could have automated 
memory map tests which had been handy in the past.

>
>> Note that in the sev_enabled() case, the "isa-bios" memory region in the 
>> -pflash
>> case will now also point to encrypted memory, just like it already does in 
>> the
>> -bios case.
>> 
>> Signed-off-by: Bernhard Beschow <shen...@gmail.com>
>> ---
>>   include/hw/i386/pc.h | 1 +
>>   hw/i386/pc.c         | 1 +
>>   hw/i386/pc_piix.c    | 3 +++
>>   hw/i386/pc_q35.c     | 2 ++
>>   hw/i386/pc_sysfw.c   | 8 +++++++-
>>   5 files changed, 14 insertions(+), 1 deletion(-)
>
>I'm still not convinced we need a migration back compat for this...

A copy behaves different than an alias, thus there is a behavioral change. 
Whether it really matters in practice for the kind of guests we care about I 
can't tell. Therefore I'd keep the compat machinery.

Best regards,
Bernhard

>
>> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
>> index 82d37cb376..ac88ad4eb9 100644
>> --- a/hw/i386/pc_sysfw.c
>> +++ b/hw/i386/pc_sysfw.c
>> @@ -135,6 +135,7 @@ static void pc_system_flash_map(PCMachineState *pcms,
>>                                   MemoryRegion *rom_memory)
>>   {
>>       X86MachineState *x86ms = X86_MACHINE(pcms);
>> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>>       hwaddr total_size = 0;
>>       int i;
>>       BlockBackend *blk;
>> @@ -184,7 +185,12 @@ static void pc_system_flash_map(PCMachineState *pcms,
>>             if (i == 0) {
>>               flash_mem = pflash_cfi01_get_memory(system_flash);
>> -            pc_isa_bios_init(&x86ms->isa_bios, rom_memory, flash_mem);
>> +            if (pcmc->isa_bios_alias) {
>> +                x86_isa_bios_init(&x86ms->isa_bios, rom_memory, flash_mem,
>> +                                  true);
>> +            } else {
>> +                pc_isa_bios_init(&x86ms->isa_bios, rom_memory, flash_mem);
>> +            }
>>                 /* Encrypt the pflash boot ROM */
>>               if (sev_enabled()) {
>

Reply via email to