Re: [Xen-devel] [PATCH v3] hvmloader, libxl: use the correct ACPI settings depending on device model

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Aug 17, 2017 at 03:57:13PM +0100, Igor Druzhinin wrote:
> We need to choose ACPI tables and ACPI IO port location
> properly depending on the device model version we are running.
> Previously, this decision was made by BIOS type specific
> code in hvmloader, e.g. always load QEMU traditional specific
> tables if it's ROMBIOS and always load QEMU Xen specific
> tables if it's SeaBIOS.
> 
> This change saves this behavior (for compatibility) but adds
> an additional way (xenstore key) to specify the correct
> device model if we happen to run a non-default one. Toolstack
> bit makes use of it.
> 
> The enforcement of BIOS type depending on QEMU version will
> be lifted later when the rest of ROMBIOS compatibility fixes
> are in place.
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
> Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> ---
> Changes in v3:
> * move ACPI table externs into util.h
> 
> Changes in v2:
> * fix insufficient allocation size of localent
> ---
>  tools/firmware/hvmloader/hvmloader.c |  2 --
>  tools/firmware/hvmloader/ovmf.c      |  5 ++---
>  tools/firmware/hvmloader/rombios.c   |  5 ++---
>  tools/firmware/hvmloader/seabios.c   |  6 +++---
>  tools/firmware/hvmloader/util.c      | 21 +++++++++++++++++++++
>  tools/firmware/hvmloader/util.h      |  3 +++
>  tools/libxl/libxl_create.c           |  4 +++-
>  7 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/hvmloader.c 
> b/tools/firmware/hvmloader/hvmloader.c
> index f603f68..db11ab1 100644
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -405,8 +405,6 @@ int main(void)
>          }
>  
>          acpi_enable_sci();
> -
> -        hvm_param_set(HVM_PARAM_ACPI_IOPORTS_LOCATION, 1);

AFAICT there's something very strange going on here. Setting
HVM_PARAM_ACPI_IOPORTS_LOCATION was done here unconditionally after
calling acpi_build_tables, which means that any selection done by the
BIOS was overwritten here.

Also the FADT has the control/event register V1 values set by default,
and they are not changed anywhere AFAICT. IMHO, hotplug for qemu-trad
has not worked in a long time I guess...

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel