All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] hvmloader, libxl: use the correct ACPI settings depending on device model
@ 2017-08-29 16:29 Igor Druzhinin
  2017-08-30  7:21 ` Roger Pau Monné
  2017-08-30  8:31 ` Jan Beulich
  0 siblings, 2 replies; 6+ messages in thread
From: Igor Druzhinin @ 2017-08-29 16:29 UTC (permalink / raw)
  To: xen-devel
  Cc: Igor Druzhinin, wei.liu2, andrew.cooper3, ian.jackson, jbeulich,
	roger.pau

We need to choose ACPI tables 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@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
---
Changes in v4:
* Use V1 port location unconditionally as modern versions of
  Qemu-trad use it anyway
* Change confusing comments in ioreq.h

Changes in v3:
* move ACPI table externs into util.h

Changes in v2:
* fix insufficient allocation size of localent
---
 tools/firmware/hvmloader/ovmf.c    |  3 ---
 tools/firmware/hvmloader/rombios.c |  3 ---
 tools/firmware/hvmloader/seabios.c |  3 ---
 tools/firmware/hvmloader/util.c    | 17 +++++++++++++++++
 tools/firmware/hvmloader/util.h    |  3 +++
 tools/libxl/libxl_create.c         |  4 +++-
 xen/include/public/hvm/ioreq.h     |  4 ++--
 7 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index 4ff7f1d..a17a11c 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -41,9 +41,6 @@
 #define LOWCHUNK_MAXOFFSET      0x0000FFFF
 #define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
 
-extern unsigned char dsdt_anycpu_qemu_xen[];
-extern int dsdt_anycpu_qemu_xen_len;
-
 #define OVMF_INFO_MAX_TABLES 4
 struct ovmf_info {
     char signature[14]; /* XenHVMOVMF\0\0\0\0 */
diff --git a/tools/firmware/hvmloader/rombios.c b/tools/firmware/hvmloader/rombios.c
index 56b39b7..c736fd9 100644
--- a/tools/firmware/hvmloader/rombios.c
+++ b/tools/firmware/hvmloader/rombios.c
@@ -42,9 +42,6 @@
 #define ROMBIOS_MAXOFFSET      0x0000FFFF
 #define ROMBIOS_END            (ROMBIOS_BEGIN + ROMBIOS_SIZE)
 
-extern unsigned char dsdt_anycpu[], dsdt_15cpu[];
-extern int dsdt_anycpu_len, dsdt_15cpu_len;
-
 static void rombios_setup_e820(void)
 {
     /*
diff --git a/tools/firmware/hvmloader/seabios.c b/tools/firmware/hvmloader/seabios.c
index 870576a..801516d 100644
--- a/tools/firmware/hvmloader/seabios.c
+++ b/tools/firmware/hvmloader/seabios.c
@@ -29,9 +29,6 @@
 #include <acpi2_0.h>
 #include <libacpi.h>
 
-extern unsigned char dsdt_anycpu_qemu_xen[];
-extern int dsdt_anycpu_qemu_xen_len;
-
 struct seabios_info {
     char signature[14]; /* XenHVMSeaBIOS\0 */
     uint8_t length;     /* Length of this struct */
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index db5f240..ab5448b 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -897,6 +897,23 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
     /* Allocate and initialise the acpi info area. */
     mem_hole_populate_ram(ACPI_INFO_PHYSICAL_ADDRESS >> PAGE_SHIFT, 1);
 
+    /* If the device model is specified switch to the corresponding tables */
+    s = xenstore_read("platform/device-model", "");
+    if ( !strncmp(s, "qemu_xen_traditional", 21) )
+    {
+        config->dsdt_anycpu = dsdt_anycpu;
+        config->dsdt_anycpu_len = dsdt_anycpu_len;
+        config->dsdt_15cpu = dsdt_15cpu;
+        config->dsdt_15cpu_len = dsdt_15cpu_len;
+    }
+    else if ( !strncmp(s, "qemu_xen", 9) )
+    {
+        config->dsdt_anycpu = dsdt_anycpu_qemu_xen;
+        config->dsdt_anycpu_len = dsdt_anycpu_qemu_xen_len;
+        config->dsdt_15cpu = NULL;
+        config->dsdt_15cpu_len = 0;
+    }
+
     config->lapic_base_address = LAPIC_BASE_ADDRESS;
     config->lapic_id = acpi_lapic_id;
     config->ioapic_base_address = ioapic_base_address;
diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index 6062f0b..874916c 100644
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -276,6 +276,9 @@ extern struct e820map memory_map;
 bool check_overlap(uint64_t start, uint64_t size,
                    uint64_t reserved_start, uint64_t reserved_size);
 
+extern unsigned char dsdt_anycpu_qemu_xen[], dsdt_anycpu[], dsdt_15cpu[];
+extern int dsdt_anycpu_qemu_xen_len, dsdt_anycpu_len, dsdt_15cpu_len;
+
 struct acpi_config;
 void hvmloader_acpi_build_tables(struct acpi_config *config,
                                  unsigned int physical);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1158303..1d24209 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -451,7 +451,7 @@ int libxl__domain_build(libxl__gc *gc,
         vments[4] = "start_time";
         vments[5] = GCSPRINTF("%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
 
-        localents = libxl__calloc(gc, 11, sizeof(char *));
+        localents = libxl__calloc(gc, 13, sizeof(char *));
         i = 0;
         localents[i++] = "platform/acpi";
         localents[i++] = libxl__acpi_defbool_val(info) ? "1" : "0";
@@ -472,6 +472,8 @@ int libxl__domain_build(libxl__gc *gc,
                                    info->u.hvm.mmio_hole_memkb << 10);
             }
         }
+        localents[i++] = "platform/device-model";
+        localents[i++] = (char *) libxl_device_model_version_to_string(info->device_model_version);
 
         break;
     case LIBXL_DOMAIN_TYPE_PV:
diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
index 2e5809b..cffee6b 100644
--- a/xen/include/public/hvm/ioreq.h
+++ b/xen/include/public/hvm/ioreq.h
@@ -103,14 +103,14 @@ typedef struct buffered_iopage buffered_iopage_t;
  * version number in HVM_PARAM_ACPI_IOPORTS_LOCATION.
  */
 
-/* Version 0 (default): Traditional Xen locations. */
+/* Version 0 (default): Traditional (obsolete) Xen locations. */
 #define ACPI_PM1A_EVT_BLK_ADDRESS_V0 0x1f40
 #define ACPI_PM1A_CNT_BLK_ADDRESS_V0 (ACPI_PM1A_EVT_BLK_ADDRESS_V0 + 0x04)
 #define ACPI_PM_TMR_BLK_ADDRESS_V0   (ACPI_PM1A_EVT_BLK_ADDRESS_V0 + 0x08)
 #define ACPI_GPE0_BLK_ADDRESS_V0     (ACPI_PM_TMR_BLK_ADDRESS_V0 + 0x20)
 #define ACPI_GPE0_BLK_LEN_V0         0x08
 
-/* Version 1: Locations preferred by modern Qemu. */
+/* Version 1: Locations preferred by modern Qemu (including Qemu-trad). */
 #define ACPI_PM1A_EVT_BLK_ADDRESS_V1 0xb000
 #define ACPI_PM1A_CNT_BLK_ADDRESS_V1 (ACPI_PM1A_EVT_BLK_ADDRESS_V1 + 0x04)
 #define ACPI_PM_TMR_BLK_ADDRESS_V1   (ACPI_PM1A_EVT_BLK_ADDRESS_V1 + 0x08)
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] hvmloader, libxl: use the correct ACPI settings depending on device model
  2017-08-29 16:29 [PATCH v4] hvmloader, libxl: use the correct ACPI settings depending on device model Igor Druzhinin
@ 2017-08-30  7:21 ` Roger Pau Monné
  2017-08-30 12:24   ` Igor Druzhinin
  2017-08-30  8:31 ` Jan Beulich
  1 sibling, 1 reply; 6+ messages in thread
From: Roger Pau Monné @ 2017-08-30  7:21 UTC (permalink / raw)
  To: Igor Druzhinin; +Cc: andrew.cooper3, ian.jackson, wei.liu2, jbeulich, xen-devel

On Tue, Aug 29, 2017 at 05:29:53PM +0100, Igor Druzhinin wrote:
> We need to choose ACPI tables 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@citrix.com>
> Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Changes in v4:
> * Use V1 port location unconditionally as modern versions of
>   Qemu-trad use it anyway
> * Change confusing comments in ioreq.h
> 
> Changes in v3:
> * move ACPI table externs into util.h
> 
> Changes in v2:
> * fix insufficient allocation size of localent
> ---
>  tools/firmware/hvmloader/ovmf.c    |  3 ---
>  tools/firmware/hvmloader/rombios.c |  3 ---
>  tools/firmware/hvmloader/seabios.c |  3 ---

You forgot to remove the calls to HVM_PARAM_ACPI_IOPORTS_LOCATION from
the above files.

> diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
> index 2e5809b..cffee6b 100644
> --- a/xen/include/public/hvm/ioreq.h
> +++ b/xen/include/public/hvm/ioreq.h
> @@ -103,14 +103,14 @@ typedef struct buffered_iopage buffered_iopage_t;
>   * version number in HVM_PARAM_ACPI_IOPORTS_LOCATION.
>   */
>  
> -/* Version 0 (default): Traditional Xen locations. */
> +/* Version 0 (default): Traditional (obsolete) Xen locations. */

Could you please add a note saying this is only keep for migration
purposes (being able to migrate from older Xen versions)?

Thanks, Roger.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] hvmloader, libxl: use the correct ACPI settings depending on device model
  2017-08-29 16:29 [PATCH v4] hvmloader, libxl: use the correct ACPI settings depending on device model Igor Druzhinin
  2017-08-30  7:21 ` Roger Pau Monné
@ 2017-08-30  8:31 ` Jan Beulich
  2017-08-30 11:24   ` Wei Liu
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2017-08-30  8:31 UTC (permalink / raw)
  To: Igor Druzhinin
  Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau

>>> On 29.08.17 at 18:29, <igor.druzhinin@citrix.com> wrote:
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -276,6 +276,9 @@ extern struct e820map memory_map;
>  bool check_overlap(uint64_t start, uint64_t size,
>                     uint64_t reserved_start, uint64_t reserved_size);
>  
> +extern unsigned char dsdt_anycpu_qemu_xen[], dsdt_anycpu[], dsdt_15cpu[];

Preferably const, unless this requires to much other code to be
changed.

> @@ -472,6 +472,8 @@ int libxl__domain_build(libxl__gc *gc,
>                                     info->u.hvm.mmio_hole_memkb << 10);
>              }
>          }
> +        localents[i++] = "platform/device-model";
> +        localents[i++] = (char *) libxl_device_model_version_to_string(info->device_model_version);

If this was hypervisor code, I'd complain about the blank following
the closing parenthesis; not sure what the libxl conventions are
in this regard.

Jan


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] hvmloader, libxl: use the correct ACPI settings depending on device model
  2017-08-30  8:31 ` Jan Beulich
@ 2017-08-30 11:24   ` Wei Liu
  0 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2017-08-30 11:24 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Igor Druzhinin, wei.liu2, andrew.cooper3, ian.jackson, xen-devel,
	roger.pau

On Wed, Aug 30, 2017 at 02:31:54AM -0600, Jan Beulich wrote:
> >>> On 29.08.17 at 18:29, <igor.druzhinin@citrix.com> wrote:
> > --- a/tools/firmware/hvmloader/util.h
> > +++ b/tools/firmware/hvmloader/util.h
> > @@ -276,6 +276,9 @@ extern struct e820map memory_map;
> >  bool check_overlap(uint64_t start, uint64_t size,
> >                     uint64_t reserved_start, uint64_t reserved_size);
> >  
> > +extern unsigned char dsdt_anycpu_qemu_xen[], dsdt_anycpu[], dsdt_15cpu[];
> 
> Preferably const, unless this requires to much other code to be
> changed.
> 
> > @@ -472,6 +472,8 @@ int libxl__domain_build(libxl__gc *gc,
> >                                     info->u.hvm.mmio_hole_memkb << 10);
> >              }
> >          }
> > +        localents[i++] = "platform/device-model";
> > +        localents[i++] = (char *) libxl_device_model_version_to_string(info->device_model_version);
> 
> If this was hypervisor code, I'd complain about the blank following
> the closing parenthesis; not sure what the libxl conventions are
> in this regard.
> 

We don't have strict requirement in CODING_STYLE. I normally would
prefer not having the space there.

I can fix that up while committing but Igor please change it if you need
to resend this patch.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] hvmloader, libxl: use the correct ACPI settings depending on device model
  2017-08-30  7:21 ` Roger Pau Monné
@ 2017-08-30 12:24   ` Igor Druzhinin
  2017-08-30 13:10     ` Roger Pau Monné
  0 siblings, 1 reply; 6+ messages in thread
From: Igor Druzhinin @ 2017-08-30 12:24 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: andrew.cooper3, ian.jackson, wei.liu2, jbeulich, xen-devel

On 30/08/17 08:21, Roger Pau Monné wrote:
> On Tue, Aug 29, 2017 at 05:29:53PM +0100, Igor Druzhinin wrote:
>> We need to choose ACPI tables 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@citrix.com>
>> Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
>> ---
>> Changes in v4:
>> * Use V1 port location unconditionally as modern versions of
>>   Qemu-trad use it anyway
>> * Change confusing comments in ioreq.h
>>
>> Changes in v3:
>> * move ACPI table externs into util.h
>>
>> Changes in v2:
>> * fix insufficient allocation size of localent
>> ---
>>  tools/firmware/hvmloader/ovmf.c    |  3 ---
>>  tools/firmware/hvmloader/rombios.c |  3 ---
>>  tools/firmware/hvmloader/seabios.c |  3 ---
> 
> You forgot to remove the calls to HVM_PARAM_ACPI_IOPORTS_LOCATION from
> the above files.
> 

I think I did that if you mean the change that I had before. These files
now don't touch HVM_PARAM_ACPI_IOPORTS_LOCATION.

>> diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
>> index 2e5809b..cffee6b 100644
>> --- a/xen/include/public/hvm/ioreq.h
>> +++ b/xen/include/public/hvm/ioreq.h
>> @@ -103,14 +103,14 @@ typedef struct buffered_iopage buffered_iopage_t;
>>   * version number in HVM_PARAM_ACPI_IOPORTS_LOCATION.
>>   */
>>  
>> -/* Version 0 (default): Traditional Xen locations. */
>> +/* Version 0 (default): Traditional (obsolete) Xen locations. */
> 
> Could you please add a note saying this is only keep for migration
> purposes (being able to migrate from older Xen versions)?
> 

Sure.

> Thanks, Roger.
> 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] hvmloader, libxl: use the correct ACPI settings depending on device model
  2017-08-30 12:24   ` Igor Druzhinin
@ 2017-08-30 13:10     ` Roger Pau Monné
  0 siblings, 0 replies; 6+ messages in thread
From: Roger Pau Monné @ 2017-08-30 13:10 UTC (permalink / raw)
  To: Igor Druzhinin; +Cc: andrew.cooper3, ian.jackson, wei.liu2, jbeulich, xen-devel

On Wed, Aug 30, 2017 at 01:24:24PM +0100, Igor Druzhinin wrote:
> On 30/08/17 08:21, Roger Pau Monné wrote:
> > On Tue, Aug 29, 2017 at 05:29:53PM +0100, Igor Druzhinin wrote:
> >>  tools/firmware/hvmloader/ovmf.c    |  3 ---
> >>  tools/firmware/hvmloader/rombios.c |  3 ---
> >>  tools/firmware/hvmloader/seabios.c |  3 ---
> > 
> > You forgot to remove the calls to HVM_PARAM_ACPI_IOPORTS_LOCATION from
> > the above files.
> > 
> 
> I think I did that if you mean the change that I had before. These files
> now don't touch HVM_PARAM_ACPI_IOPORTS_LOCATION.

Sorry, I was looking at the code without the revert.

Roger.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-08-30 13:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-29 16:29 [PATCH v4] hvmloader, libxl: use the correct ACPI settings depending on device model Igor Druzhinin
2017-08-30  7:21 ` Roger Pau Monné
2017-08-30 12:24   ` Igor Druzhinin
2017-08-30 13:10     ` Roger Pau Monné
2017-08-30  8:31 ` Jan Beulich
2017-08-30 11:24   ` Wei Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.