mbox series

[0/4] X86: Alias isa-bios area and clean up

Message ID 20240422200625.2768-1-shentey@gmail.com
Headers show
Series X86: Alias isa-bios area and clean up | expand

Message

Bernhard Beschow April 22, 2024, 8:06 p.m. UTC
This series changes the "isa-bios" MemoryRegion to be an alias rather than a
copy in the pflash case. This fixes issuing pflash commands in the isa-bios
region which matches real hardware and which some real-world legacy bioses I'm
running rely on. Furthermore, aliasing in the isa-bios area is already the
current behavior in the bios (a.k.a. ROM) case, so this series consolidates
behavior.

The consolidateion results in duplicate code which is resolved in the second
half (patches 3 and 4) in this series.

Question: AFAIU, patch 2 changes the behavior for SEV-enabled guests since the
isa-bios area is now encrypted. Does this need compat machinery or is it a
bugfix?

Testing done:
* `make check` with qemu-system-x86_64 (QEMU 8.2.2) installed. All tests
  including migration tests pass.
* `make check-avocado`

Best regards,
Bernhard

Bernhard Beschow (4):
  hw/i386/pc_sysfw: Remove unused parameter from pc_isa_bios_init()
  hw/i386/pc_sysfw: Alias rather than copy isa-bios region
  hw/i386/x86: Eliminate two if statements in x86_bios_rom_init()
  hw/i386: Consolidate isa-bios creation

 include/hw/i386/x86.h |  2 ++
 hw/i386/pc_sysfw.c    | 38 ++++----------------------------------
 hw/i386/x86.c         | 35 +++++++++++++++++++----------------
 3 files changed, 25 insertions(+), 50 deletions(-)

Comments

Bernhard Beschow April 24, 2024, 8:05 p.m. UTC | #1
+Michael 

Am 22. April 2024 20:06:21 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>This series changes the "isa-bios" MemoryRegion to be an alias rather than a
>
>copy in the pflash case. This fixes issuing pflash commands in the isa-bios
>
>region which matches real hardware and which some real-world legacy bioses I'm
>
>running rely on. Furthermore, aliasing in the isa-bios area is already the
>
>current behavior in the bios (a.k.a. ROM) case, so this series consolidates
>
>behavior.
>
>
>
>The consolidateion results in duplicate code which is resolved in the second
>
>half (patches 3 and 4) in this series.
>
>
>
>Question: AFAIU, patch 2 changes the behavior for SEV-enabled guests since the
>
>isa-bios area is now encrypted. Does this need compat machinery or is it a
>
>bugfix?
>
>
>
>Testing done:
>
>* `make check` with qemu-system-x86_64 (QEMU 8.2.2) installed. All tests
>
>  including migration tests pass.
>
>* `make check-avocado`
>
>
>
>Best regards,
>
>Bernhard
>
>
>
>Bernhard Beschow (4):
>
>  hw/i386/pc_sysfw: Remove unused parameter from pc_isa_bios_init()
>
>  hw/i386/pc_sysfw: Alias rather than copy isa-bios region
>
>  hw/i386/x86: Eliminate two if statements in x86_bios_rom_init()
>
>  hw/i386: Consolidate isa-bios creation
>
>
>
> include/hw/i386/x86.h |  2 ++
>
> hw/i386/pc_sysfw.c    | 38 ++++----------------------------------
>
> hw/i386/x86.c         | 35 +++++++++++++++++++----------------
>
> 3 files changed, 25 insertions(+), 50 deletions(-)
>
>
>
>-- >
>2.44.0
>
>
>
Philippe Mathieu-Daudé April 25, 2024, 8:07 a.m. UTC | #2
On 22/4/24 22:06, Bernhard Beschow wrote:
> This series changes the "isa-bios" MemoryRegion to be an alias rather than a
> copy in the pflash case. This fixes issuing pflash commands in the isa-bios
> region which matches real hardware and which some real-world legacy bioses I'm
> running rely on. Furthermore, aliasing in the isa-bios area is already the
> current behavior in the bios (a.k.a. ROM) case, so this series consolidates
> behavior.


> Bernhard Beschow (4):
>    hw/i386/pc_sysfw: Remove unused parameter from pc_isa_bios_init()

To reduce respin churn, I'm queuing the first patch via hw-misc.
Michael S. Tsirkin April 25, 2024, 10:16 a.m. UTC | #3
On Mon, Apr 22, 2024 at 10:06:21PM +0200, Bernhard Beschow wrote:
> This series changes the "isa-bios" MemoryRegion to be an alias rather than a
> copy in the pflash case. This fixes issuing pflash commands in the isa-bios
> region which matches real hardware and which some real-world legacy bioses I'm
> running rely on. Furthermore, aliasing in the isa-bios area is already the
> current behavior in the bios (a.k.a. ROM) case, so this series consolidates
> behavior.
> 
> The consolidateion results in duplicate code which is resolved in the second
> half (patches 3 and 4) in this series.
> 
> Question: AFAIU, patch 2 changes the behavior for SEV-enabled guests since the
> isa-bios area is now encrypted. Does this need compat machinery or is it a
> bugfix?

When in doubt, do a compat thing.

> Testing done:
> * `make check` with qemu-system-x86_64 (QEMU 8.2.2) installed. All tests
>   including migration tests pass.
> * `make check-avocado`
> 
> Best regards,
> Bernhard
> 
> Bernhard Beschow (4):
>   hw/i386/pc_sysfw: Remove unused parameter from pc_isa_bios_init()
>   hw/i386/pc_sysfw: Alias rather than copy isa-bios region
>   hw/i386/x86: Eliminate two if statements in x86_bios_rom_init()
>   hw/i386: Consolidate isa-bios creation
> 
>  include/hw/i386/x86.h |  2 ++
>  hw/i386/pc_sysfw.c    | 38 ++++----------------------------------
>  hw/i386/x86.c         | 35 +++++++++++++++++++----------------
>  3 files changed, 25 insertions(+), 50 deletions(-)
> 
> -- 
> 2.44.0
>
Bernhard Beschow April 26, 2024, 1:05 p.m. UTC | #4
Am 25. April 2024 10:16:10 UTC schrieb "Michael S. Tsirkin" <mst@redhat.com>:
>On Mon, Apr 22, 2024 at 10:06:21PM +0200, Bernhard Beschow wrote:
>> This series changes the "isa-bios" MemoryRegion to be an alias rather than a
>> copy in the pflash case. This fixes issuing pflash commands in the isa-bios
>> region which matches real hardware and which some real-world legacy bioses I'm
>> running rely on. Furthermore, aliasing in the isa-bios area is already the
>> current behavior in the bios (a.k.a. ROM) case, so this series consolidates
>> behavior.
>> 
>> The consolidateion results in duplicate code which is resolved in the second
>> half (patches 3 and 4) in this series.
>> 
>> Question: AFAIU, patch 2 changes the behavior for SEV-enabled guests since the
>> isa-bios area is now encrypted. Does this need compat machinery or is it a
>> bugfix?
>
>When in doubt, do a compat thing.

Will do.

Thanks,
Bernhard

>
>> Testing done:
>> * `make check` with qemu-system-x86_64 (QEMU 8.2.2) installed. All tests
>>   including migration tests pass.
>> * `make check-avocado`
>> 
>> Best regards,
>> Bernhard
>> 
>> Bernhard Beschow (4):
>>   hw/i386/pc_sysfw: Remove unused parameter from pc_isa_bios_init()
>>   hw/i386/pc_sysfw: Alias rather than copy isa-bios region
>>   hw/i386/x86: Eliminate two if statements in x86_bios_rom_init()
>>   hw/i386: Consolidate isa-bios creation
>> 
>>  include/hw/i386/x86.h |  2 ++
>>  hw/i386/pc_sysfw.c    | 38 ++++----------------------------------
>>  hw/i386/x86.c         | 35 +++++++++++++++++++----------------
>>  3 files changed, 25 insertions(+), 50 deletions(-)
>> 
>> -- 
>> 2.44.0
>> 
>
Bernhard Beschow April 26, 2024, 1:09 p.m. UTC | #5
Am 25. April 2024 08:07:43 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 22/4/24 22:06, Bernhard Beschow wrote:
>> This series changes the "isa-bios" MemoryRegion to be an alias rather than a
>> copy in the pflash case. This fixes issuing pflash commands in the isa-bios
>> region which matches real hardware and which some real-world legacy bioses I'm
>> running rely on. Furthermore, aliasing in the isa-bios area is already the
>> current behavior in the bios (a.k.a. ROM) case, so this series consolidates
>> behavior.
>
>
>> Bernhard Beschow (4):
>>    hw/i386/pc_sysfw: Remove unused parameter from pc_isa_bios_init()
>
>To reduce respin churn, I'm queuing the first patch via hw-misc.

Nice!