anthony.perard@citrix.com
2010-Oct-29 12:08 UTC
[Xen-devel] [PATCH V3 0/6] firmware changes as part of QEMU/Xen merge.
From: Anthony PERARD <anthony.perard@citrix.com> This patches are for Xen-unstable and Qemu-DM. With the integration of Xen into QEMU, we would like to minimise the change and use the ACPI implementation of QEMU. So there are some change to make on the firmware to match the QEMU''s BIOS. Some IO Ports are different and the sleep state values are different. Change with the v2->v3: Xen will register the old ioport by default and switch to the new ioport if the HVM_PARAM_ACPI_NEW_IOPORT is set. So, this series come with a new function, unregister_io_handler, and a new HVM_PARAM. Change v1->v2: This time, both old and new ioport are handled. In QEMU-Xen, the choice between one or the other is made when a saved state is restored. In Xen, both ioports are registered. Anthony PERARD (5): firmware, Change ACPI IO addresses and values to match QEMU BIOS. xen, Introduce unregister_io_handler xen, Intruduce pmtimer_change_ioport and HVM_PARAM_ACPI_NEW_IOPORT. firmware, Set HVM_PARAM_ACPI_NEW_IOPORT libxc, save/restore, Save the HVM_PARAM_ACPI_NEW_IOPORT tools/firmware/hvmloader/acpi/dsdt.asl | 12 ++++++------ tools/firmware/hvmloader/hvmloader.c | 11 +++++++++++ tools/libxc/xc_domain_restore.c | 12 ++++++++++++ tools/libxc/xc_domain_save.c | 11 +++++++++++ tools/libxc/xg_save_restore.h | 1 + xen/arch/x86/hvm/hvm.c | 9 +++++++++ xen/arch/x86/hvm/intercept.c | 26 ++++++++++++++++++++++++++ xen/arch/x86/hvm/pmtimer.c | 26 ++++++++++++++++++++++++-- xen/include/asm-x86/hvm/io.h | 8 ++++++++ xen/include/asm-x86/hvm/vpt.h | 1 + xen/include/public/hvm/ioreq.h | 16 +++++++++++++--- xen/include/public/hvm/params.h | 5 ++++- 12 files changed, 126 insertions(+), 12 deletions(-) QEMU-Xen change: Anthony PERARD (1): piix4acpi: change in ACPI to match the change in the BIOS. hw/piix4acpi.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 82 insertions(+), 17 deletions(-) -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Oct-29 12:08 UTC
[Xen-devel] [PATCH V3 1/6] firmware, Change ACPI IO addresses and values to match QEMU BIOS.
From: Anthony PERARD <anthony.perard@citrix.com>
As part of the QEMU/Xen merge, this patch comes to change the IO Port
adresses, the value of sleep states and add some information in the PCI
registers to match the implementation of the BIOS of QEMU.
This patch must be applied at the same time with an other patch for the
piix4acpi of qemu-xen.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/firmware/hvmloader/acpi/dsdt.asl | 12 ++++++------
tools/firmware/hvmloader/hvmloader.c | 4 ++++
xen/include/public/hvm/ioreq.h | 16 +++++++++++++---
3 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/tools/firmware/hvmloader/acpi/dsdt.asl
b/tools/firmware/hvmloader/acpi/dsdt.asl
index 03799d3..ea8e324 100644
--- a/tools/firmware/hvmloader/acpi/dsdt.asl
+++ b/tools/firmware/hvmloader/acpi/dsdt.asl
@@ -33,22 +33,22 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2,
"Xen", "HVM", 0)
*/
Name (\_S3, Package (0x04)
{
- 0x05, /* PM1a_CNT.SLP_TYP */
- 0x05, /* PM1b_CNT.SLP_TYP */
+ 0x01, /* PM1a_CNT.SLP_TYP */
+ 0x01, /* PM1b_CNT.SLP_TYP */
0x0, /* reserved */
0x0 /* reserved */
})
Name (\_S4, Package (0x04)
{
- 0x06, /* PM1a_CNT.SLP_TYP */
- 0x06, /* PM1b_CNT.SLP_TYP */
+ 0x00, /* PM1a_CNT.SLP_TYP */
+ 0x00, /* PM1b_CNT.SLP_TYP */
0x00, /* reserved */
0x00 /* reserved */
})
Name (\_S5, Package (0x04)
{
- 0x07, /* PM1a_CNT.SLP_TYP */
- 0x07, /* PM1b_CNT.SLP_TYP */
+ 0x00, /* PM1a_CNT.SLP_TYP */
+ 0x00, /* PM1b_CNT.SLP_TYP */
0x00, /* reserved */
0x00 /* reserved */
})
diff --git a/tools/firmware/hvmloader/hvmloader.c
b/tools/firmware/hvmloader/hvmloader.c
index c4c5ddc..cfe026f 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -31,6 +31,7 @@
#include "option_rom.h"
#include <xen/version.h>
#include <xen/hvm/params.h>
+#include <xen/hvm/ioreq.h>
#include <xen/memory.h>
asm (
@@ -222,9 +223,12 @@ static void pci_setup(void)
/* PIIX4 ACPI PM. Special device with special PCI config space. */
ASSERT((vendor_id == 0x8086) && (device_id == 0x7113));
pci_writew(devfn, 0x20, 0x0000); /* No smb bus IO enable */
+ pci_writew(devfn, 0xd2, 0x0000); /* No smb bus IO enable */
pci_writew(devfn, 0x22, 0x0000);
pci_writew(devfn, 0x3c, 0x0009); /* Hardcoded IRQ9 */
pci_writew(devfn, 0x3d, 0x0001);
+ pci_writel(devfn, 0x40, ACPI_PM1A_EVT_BLK_ADDRESS | 1);
+ pci_writeb(devfn, 0x80, 0x01); /* enable PM io space */
break;
case 0x0101:
if ( vendor_id == 0x8086 )
diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
index 0c10c08..8c11767 100644
--- a/xen/include/public/hvm/ioreq.h
+++ b/xen/include/public/hvm/ioreq.h
@@ -100,11 +100,21 @@ struct buffered_piopage {
};
#endif /* defined(__ia64__) */
-#define ACPI_PM1A_EVT_BLK_ADDRESS 0x0000000000001f40
+/*
+ * Value used by old qemu-dm, there have been replace to match
+ * the QEMU BIOS.
+ */
+#define ACPI_PM1A_EVT_BLK_ADDRESS_OLD 0x0000000000001f40
+#define ACPI_PM1A_CNT_BLK_ADDRESS_OLD (ACPI_PM1A_EVT_BLK_ADDRESS_OLD +
0x04)
+#define ACPI_PM_TMR_BLK_ADDRESS_OLD (ACPI_PM1A_EVT_BLK_ADDRESS_OLD +
0x08)
+#define ACPI_GPE0_BLK_ADDRESS_OLD (ACPI_PM_TMR_BLK_ADDRESS_OLD +
0x20)
+#define ACPI_GPE0_BLK_LEN_OLD 0x08
+
+#define ACPI_PM1A_EVT_BLK_ADDRESS 0x000000000000b000
#define ACPI_PM1A_CNT_BLK_ADDRESS (ACPI_PM1A_EVT_BLK_ADDRESS + 0x04)
#define ACPI_PM_TMR_BLK_ADDRESS (ACPI_PM1A_EVT_BLK_ADDRESS + 0x08)
-#define ACPI_GPE0_BLK_ADDRESS (ACPI_PM_TMR_BLK_ADDRESS + 0x20)
-#define ACPI_GPE0_BLK_LEN 0x08
+#define ACPI_GPE0_BLK_ADDRESS (0x000000000000afe0)
+#define ACPI_GPE0_BLK_LEN 0x04
#endif /* _IOREQ_H_ */
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Oct-29 12:08 UTC
[Xen-devel] [PATCH V3 2/6] xen, Introduce unregister_io_handler
From: Anthony PERARD <anthony.perard@citrix.com>
To remove a handler of the io handler list, by replacing it with the
last one.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
xen/arch/x86/hvm/intercept.c | 26 ++++++++++++++++++++++++++
xen/include/asm-x86/hvm/io.h | 8 ++++++++
2 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/xen/arch/x86/hvm/intercept.c b/xen/arch/x86/hvm/intercept.c
index 4af9e3d..93dffc2 100644
--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -244,6 +244,32 @@ void register_io_handler(
handler->num_slot++;
}
+void unregister_io_handler(
+ struct domain *d, unsigned long addr, unsigned long size, int type)
+{
+ struct hvm_io_handler *handler = &d->arch.hvm_domain.io_handler;
+ int last = handler->num_slot - 1;
+ int i = 0;
+
+ for (i = 0; i <= last; i++) {
+ if (handler->hdl_list[i].addr == addr
+ && handler->hdl_list[i].size == size
+ && handler->hdl_list[i].type == type) {
+
+ handler->hdl_list[i].addr = handler->hdl_list[last].addr;
+ handler->hdl_list[i].size = handler->hdl_list[last].size;
+ handler->hdl_list[i].type = handler->hdl_list[last].type;
+ if (handler->hdl_list[i].type == HVM_PORTIO) {
+ handler->hdl_list[i].action.portio =
handler->hdl_list[last].action.portio;
+ } else {
+ handler->hdl_list[i].action.mmio =
handler->hdl_list[last].action.mmio;
+ }
+ handler->num_slot--;
+ return;
+ }
+ }
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
index c10a0be..2ce8bc9 100644
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -68,6 +68,8 @@ int hvm_io_intercept(ioreq_t *p, int type);
void register_io_handler(
struct domain *d, unsigned long addr, unsigned long size,
void *action, int type);
+void unregister_io_handler(
+ struct domain *d, unsigned long addr, unsigned long size, int type);
static inline int hvm_portio_intercept(ioreq_t *p)
{
@@ -89,6 +91,12 @@ static inline void register_portio_handler(
register_io_handler(d, addr, size, action, HVM_PORTIO);
}
+static inline void unregister_portio_handler(
+ struct domain *d, unsigned long addr, unsigned long size)
+{
+ unregister_io_handler(d, addr, size, HVM_PORTIO);
+}
+
static inline void register_buffered_io_handler(
struct domain *d, unsigned long addr,
unsigned long size, mmio_action_t action)
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Oct-29 12:08 UTC
[Xen-devel] [PATCH V3 3/6] xen, Intruduce pmtimer_change_ioport and HVM_PARAM_ACPI_NEW_IOPORT.
From: Anthony PERARD <anthony.perard@citrix.com>
By default, Xen will handle the old ACPI IO port. But it can switch to
the new one by setting the HVM_PARAM_ACPI_NEW_IOPORT to 1.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
xen/arch/x86/hvm/hvm.c | 9 +++++++++
xen/arch/x86/hvm/pmtimer.c | 26 ++++++++++++++++++++++++--
xen/include/asm-x86/hvm/vpt.h | 1 +
xen/include/public/hvm/params.h | 5 ++++-
4 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 94190d3..ea296e5 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2991,6 +2991,15 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void)
arg)
rc = -EINVAL;
break;
+ case HVM_PARAM_ACPI_NEW_IOPORT:
+ if (a.value == 1)
+ pmtimer_change_ioport(d, 1);
+ else if (a.value == 0)
+ pmtimer_change_ioport(d, 0);
+ else
+ rc = -EINVAL;
+
+ break;
}
if ( rc == 0 )
diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c
index 1b9ab7b..f046201 100644
--- a/xen/arch/x86/hvm/pmtimer.c
+++ b/xen/arch/x86/hvm/pmtimer.c
@@ -24,6 +24,9 @@
#include <asm/acpi.h> /* for hvm_acpi_power_button prototype */
/* Slightly more readable port I/O addresses for the registers we intercept */
+#define PM1a_STS_ADDR_OLD (ACPI_PM1A_EVT_BLK_ADDRESS_OLD)
+#define PM1a_EN_ADDR_OLD (ACPI_PM1A_EVT_BLK_ADDRESS_OLD + 2)
+#define TMR_VAL_ADDR_OLD (ACPI_PM_TMR_BLK_ADDRESS_OLD)
#define PM1a_STS_ADDR (ACPI_PM1A_EVT_BLK_ADDRESS)
#define PM1a_EN_ADDR (ACPI_PM1A_EVT_BLK_ADDRESS + 2)
#define TMR_VAL_ADDR (ACPI_PM_TMR_BLK_ADDRESS)
@@ -155,16 +158,20 @@ static int handle_evt_io(
switch ( addr )
{
/* PM1a_STS register bits are write-to-clear */
+ case PM1a_STS_ADDR_OLD:
case PM1a_STS_ADDR:
s->pm.pm1a_sts &= ~byte;
break;
+ case PM1a_STS_ADDR_OLD + 1:
case PM1a_STS_ADDR + 1:
s->pm.pm1a_sts &= ~(byte << 8);
break;
+ case PM1a_EN_ADDR_OLD:
case PM1a_EN_ADDR:
s->pm.pm1a_en = (s->pm.pm1a_en & 0xff00) | byte;
break;
+ case PM1a_EN_ADDR_OLD + 1:
case PM1a_EN_ADDR + 1:
s->pm.pm1a_en = (s->pm.pm1a_en & 0xff) | (byte
<< 8);
break;
@@ -272,6 +279,21 @@ static int pmtimer_load(struct domain *d,
hvm_domain_context_t *h)
HVM_REGISTER_SAVE_RESTORE(PMTIMER, pmtimer_save, pmtimer_load,
1, HVMSR_PER_DOM);
+void pmtimer_change_ioport(struct domain *d, int use_new)
+{
+ if (use_new) {
+ register_portio_handler(d, TMR_VAL_ADDR, 4, handle_pmt_io);
+ register_portio_handler(d, PM1a_STS_ADDR, 4, handle_evt_io);
+ unregister_portio_handler(d, TMR_VAL_ADDR_OLD, 4);
+ unregister_portio_handler(d, PM1a_STS_ADDR_OLD, 4);
+ } else {
+ register_portio_handler(d, TMR_VAL_ADDR_OLD, 4, handle_pmt_io);
+ register_portio_handler(d, PM1a_STS_ADDR_OLD, 4, handle_evt_io);
+ unregister_portio_handler(d, TMR_VAL_ADDR, 4);
+ unregister_portio_handler(d, PM1a_STS_ADDR, 4);
+ }
+}
+
void pmtimer_init(struct vcpu *v)
{
PMTState *s = &v->domain->arch.hvm_domain.pl_time.vpmt;
@@ -284,8 +306,8 @@ void pmtimer_init(struct vcpu *v)
/* Intercept port I/O (need two handlers because PM1a_CNT is between
* PM1a_EN and TMR_VAL and is handled by qemu) */
- register_portio_handler(v->domain, TMR_VAL_ADDR, 4, handle_pmt_io);
- register_portio_handler(v->domain, PM1a_STS_ADDR, 4, handle_evt_io);
+ register_portio_handler(v->domain, TMR_VAL_ADDR_OLD, 4, handle_pmt_io);
+ register_portio_handler(v->domain, PM1a_STS_ADDR_OLD, 4, handle_evt_io);
/* Set up callback to fire SCIs when the MSB of TMR_VAL changes */
init_timer(&s->timer, pmt_timer_callback, s, v->processor);
diff --git a/xen/include/asm-x86/hvm/vpt.h b/xen/include/asm-x86/hvm/vpt.h
index 65b0dff..6b68888 100644
--- a/xen/include/asm-x86/hvm/vpt.h
+++ b/xen/include/asm-x86/hvm/vpt.h
@@ -179,6 +179,7 @@ void rtc_update_clock(struct domain *d);
void pmtimer_init(struct vcpu *v);
void pmtimer_deinit(struct domain *d);
void pmtimer_reset(struct domain *d);
+void pmtimer_change_ioport(struct domain *d, int use_new);
void hpet_init(struct vcpu *v);
void hpet_deinit(struct domain *d);
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 673148b..40af8d8 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -113,6 +113,9 @@
#define HVM_PARAM_CONSOLE_PFN 17
#define HVM_PARAM_CONSOLE_EVTCHN 18
-#define HVM_NR_PARAMS 19
+/* to specify which firmware acpi ioport is used */
+#define HVM_PARAM_ACPI_NEW_IOPORT 19
+
+#define HVM_NR_PARAMS 20
#endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Oct-29 12:08 UTC
[Xen-devel] [PATCH V3 4/6] firmware, Set HVM_PARAM_ACPI_NEW_IOPORT
From: Anthony PERARD <anthony.perard@citrix.com>
This will tell the Xen to use the new Port I/O instead of the old one.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/firmware/hvmloader/hvmloader.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/tools/firmware/hvmloader/hvmloader.c
b/tools/firmware/hvmloader/hvmloader.c
index cfe026f..28481f0 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -763,8 +763,15 @@ int main(void)
if ( hvm_info->acpi_enabled )
{
+ struct xen_hvm_param p = {
+ .domid = DOMID_SELF,
+ .index = HVM_PARAM_ACPI_NEW_IOPORT,
+ .value = 1,
+ };
+
printf("Loading ACPI ...\n");
acpi_build_tables();
+ hypercall_hvm_op(HVMOP_set_param, &p);
}
init_vm86_tss();
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Oct-29 12:08 UTC
[Xen-devel] [PATCH V3 5/6] libxc, save/restore, Save the HVM_PARAM_ACPI_NEW_IOPORT
From: Anthony PERARD <anthony.perard@citrix.com>
This will save the acpi_new_ioport hvm_param in the checkpoint file and
set the parameter in Xen at restore.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxc/xc_domain_restore.c | 12 ++++++++++++
tools/libxc/xc_domain_save.c | 11 +++++++++++
tools/libxc/xg_save_restore.h | 1 +
3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index 316f32d..c03512f 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -647,6 +647,7 @@ typedef struct {
uint64_t identpt;
uint64_t vm86_tss;
uint64_t console_pfn;
+ int new_acpi_ioport;
} pagebuf_t;
static int pagebuf_init(pagebuf_t* buf)
@@ -771,6 +772,10 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
// DPRINTF("last checkpoint indication received");
return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ case XC_SAVE_ID_HVM_ACPI_NEW_IOPORT:
+ buf->new_acpi_ioport = 1;
+ return pagebuf_get_one(xch, ctx, buf, fd, dom);
+
default:
if ( (count > MAX_BATCH_SIZE) || (count < 0) ) {
ERROR("Max batch size exceeded (%d). Giving up.", count);
@@ -1317,6 +1322,13 @@ int xc_domain_restore(xc_interface *xch, int io_fd,
uint32_t dom,
fcntl(io_fd, F_SETFL, orig_io_fd_flags | O_NONBLOCK);
}
+ if (pagebuf.new_acpi_ioport) {
+ DBGPRINTF("Use new firmware ioport from the checkpoint\n");
+ xc_set_hvm_param(xch, dom, HVM_PARAM_ACPI_NEW_IOPORT, 1);
+ } else {
+ DBGPRINTF("Use old firmware ioport from the checkpoint\n");
+ }
+
if ( ctx->last_checkpoint )
{
// DPRINTF("Last checkpoint, finishing\n");
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index 47f8a79..83af771 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -1618,6 +1618,17 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t
dom, uint32_t max_iter
PERROR("Error when writing the console pfn for guest");
goto out;
}
+
+ chunk.id = XC_SAVE_ID_HVM_ACPI_NEW_IOPORT;
+ xc_get_hvm_param(xch, dom, HVM_PARAM_ACPI_NEW_IOPORT,
+ (unsigned long *)&chunk.data);
+
+ if ( (chunk.data != 0) &&
+ wrexact(io_fd, &chunk.id, sizeof(int)) )
+ {
+ PERROR("Error when writing the firmware ioport version");
+ goto out;
+ }
}
if ( !callbacks->checkpoint )
diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h
index 2c82ce7..c86b6c4 100644
--- a/tools/libxc/xg_save_restore.h
+++ b/tools/libxc/xg_save_restore.h
@@ -132,6 +132,7 @@
#define XC_SAVE_ID_TSC_INFO -7
#define XC_SAVE_ID_HVM_CONSOLE_PFN -8 /* (HVM-only) */
#define XC_SAVE_ID_LAST_CHECKPOINT -9 /* Commit to restoring after
completion of current iteration. */
+#define XC_SAVE_ID_HVM_ACPI_NEW_IOPORT -10
/*
** We process save/restore/migrate in batches of pages; the below
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Oct-29 12:08 UTC
[Xen-devel] [PATCH V3 6/6] piix4acpi: change in ACPI to match the change in the BIOS.
From: Anthony PERARD <anthony.perard@citrix.com>
Some change have been introduced in the firmware to match QEMU''s BIOS.
So this patch adds the new sleep state values and handle old and new
ACPI IOPort mapping.
QEMU-Xen uses new ioport by default, but if it''s a saved state with old
firmware, it unmaps the new ioport and maps the old one.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
hw/piix4acpi.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 82 insertions(+), 17 deletions(-)
diff --git a/hw/piix4acpi.c b/hw/piix4acpi.c
index 1efa77d..92e728e 100644
--- a/hw/piix4acpi.c
+++ b/hw/piix4acpi.c
@@ -52,9 +52,12 @@
/* Sleep state type codes as defined by the \_Sx objects in the DSDT. */
/* These must be kept in sync with the DSDT (hvmloader/acpi/dsdt.asl) */
-#define SLP_TYP_S4 (6 << 10)
-#define SLP_TYP_S3 (5 << 10)
-#define SLP_TYP_S5 (7 << 10)
+#define SLP_TYP_S4_OLD (6 << 10)
+#define SLP_TYP_S3_OLD (5 << 10)
+#define SLP_TYP_S5_OLD (7 << 10)
+#define SLP_TYP_S4 (0 << 10)
+#define SLP_TYP_S3 (1 << 10)
+#define SLP_TYP_S5 (0 << 10)
#define ACPI_DBG_IO_ADDR 0xb044
#define ACPI_PHP_IO_ADDR 0x10c0
@@ -75,12 +78,15 @@
typedef struct PCIAcpiState {
PCIDevice dev;
uint16_t pm1_control; /* pm1a_ECNT_BLK */
+
+ /* if true, use old ioport of the firmware. */
+ uint8_t use_old_ioport;
} PCIAcpiState;
typedef struct GPEState {
/* GPE0 block */
- uint8_t gpe0_sts[ACPI_GPE0_BLK_LEN / 2];
- uint8_t gpe0_en[ACPI_GPE0_BLK_LEN / 2];
+ uint8_t gpe0_sts[ACPI_GPE0_BLK_LEN_OLD / 2];
+ uint8_t gpe0_en[ACPI_GPE0_BLK_LEN_OLD / 2];
/* CPU bitmap */
uint8_t cpus_sts[32];
@@ -88,6 +94,8 @@ typedef struct GPEState {
/* SCI IRQ level */
uint8_t sci_asserted;
+ /* if true, use old ioport of the firmware. */
+ uint8_t use_old_ioport;
} GPEState;
static GPEState gpe_state;
@@ -100,6 +108,9 @@ typedef struct PHPDevFn {
* PSTB in ASL */
} PHPDevFn;
+static void acpi_map(PCIDevice *pci_dev, int region_num,
+ uint32_t addr, uint32_t size, int type);
+
static PHPDevFn php_devfn;
int s3_shutdown_flag;
static qemu_irq sci_irq;
@@ -138,18 +149,36 @@ static void piix4acpi_save(QEMUFile *f, void *opaque)
PCIAcpiState *s = opaque;
pci_device_save(&s->dev, f);
qemu_put_be16s(f, &s->pm1_control);
+ qemu_put_8s(f, &s->use_old_ioport);
}
static int piix4acpi_load(QEMUFile *f, void *opaque, int version_id)
{
PCIAcpiState *s = opaque;
int ret;
- if (version_id > 1)
+
+ if (version_id > 2)
return -EINVAL;
ret = pci_device_load(&s->dev, f);
if (ret < 0)
return ret;
qemu_get_be16s(f, &s->pm1_control);
+
+ if (version_id <= 1) {
+ /* map to old ioport instead of the new one */
+ s->use_old_ioport = 1;
+ } else {
+ qemu_get_8s(f, &s->use_old_ioport);
+ }
+
+ if (s->use_old_ioport) {
+ PIIX4ACPI_LOG(PIIX4ACPI_LOG_DEBUG, "ACPI: Use old firmware
IOPorts.\n");
+ /* unmap new ioport to use old ioport */
+ isa_unassign_ioport(ACPI_PM1A_EVT_BLK_ADDRESS + 4, 2);
+ acpi_map((PCIDevice *)s, 0, ACPI_PM1A_EVT_BLK_ADDRESS_OLD, 0x10,
PCI_ADDRESS_SPACE_IO);
+ } else {
+ PIIX4ACPI_LOG(PIIX4ACPI_LOG_DEBUG, "ACPI: Keep new firmware
IOPorts.\n");
+ }
return 0;
}
@@ -172,6 +201,7 @@ static void acpi_shutdown(uint32_t val)
return;
switch (val & SLP_TYP_Sx) {
+ case SLP_TYP_S3_OLD:
case SLP_TYP_S3:
s3_shutdown_flag = 1;
qemu_system_reset();
@@ -179,7 +209,8 @@ static void acpi_shutdown(uint32_t val)
cmos_set_s3_resume();
xc_set_hvm_param(xc_handle, domid, HVM_PARAM_ACPI_S_STATE, 3);
break;
- case SLP_TYP_S4:
+ case SLP_TYP_S4_OLD:
+ case SLP_TYP_S5_OLD:
case SLP_TYP_S5:
qemu_system_shutdown_request();
break;
@@ -403,7 +434,10 @@ static uint32_t gpe_sts_read(void *opaque, uint32_t addr)
{
GPEState *s = opaque;
- return s->gpe0_sts[addr - ACPI_GPE0_BLK_ADDRESS];
+ if (s->use_old_ioport)
+ return s->gpe0_sts[addr - ACPI_GPE0_BLK_ADDRESS_OLD];
+ else
+ return s->gpe0_sts[addr - ACPI_GPE0_BLK_ADDRESS];
}
/* write 1 to clear specific GPE bits */
@@ -415,7 +449,10 @@ static void gpe_sts_write(void *opaque, uint32_t addr,
uint32_t val)
PIIX4ACPI_LOG(PIIX4ACPI_LOG_DEBUG, "gpe_sts_write: addr=0x%x,
val=0x%x.\n", addr, val);
hotplugged = test_bit(&s->gpe0_sts[0], ACPI_PHP_GPE_BIT);
- s->gpe0_sts[addr - ACPI_GPE0_BLK_ADDRESS] &= ~val;
+ if (s->use_old_ioport)
+ s->gpe0_sts[addr - ACPI_GPE0_BLK_ADDRESS_OLD] &= ~val;
+ else
+ s->gpe0_sts[addr - ACPI_GPE0_BLK_ADDRESS] &= ~val;
if ( s->sci_asserted &&
hotplugged &&
!test_bit(&s->gpe0_sts[0], ACPI_PHP_GPE_BIT)) {
@@ -429,7 +466,10 @@ static uint32_t gpe_en_read(void *opaque, uint32_t addr)
{
GPEState *s = opaque;
- return s->gpe0_en[addr - (ACPI_GPE0_BLK_ADDRESS + ACPI_GPE0_BLK_LEN /
2)];
+ if (s->use_old_ioport)
+ return s->gpe0_en[addr - (ACPI_GPE0_BLK_ADDRESS_OLD +
ACPI_GPE0_BLK_LEN_OLD / 2)];
+ else
+ return s->gpe0_en[addr - (ACPI_GPE0_BLK_ADDRESS + ACPI_GPE0_BLK_LEN
/ 2)];
}
/* write 0 to clear en bit */
@@ -439,7 +479,10 @@ static void gpe_en_write(void *opaque, uint32_t addr,
uint32_t val)
int reg_count;
PIIX4ACPI_LOG(PIIX4ACPI_LOG_DEBUG, "gpe_en_write: addr=0x%x,
val=0x%x.\n", addr, val);
- reg_count = addr - (ACPI_GPE0_BLK_ADDRESS + ACPI_GPE0_BLK_LEN / 2);
+ if (s->use_old_ioport)
+ reg_count = addr - (ACPI_GPE0_BLK_ADDRESS_OLD + ACPI_GPE0_BLK_LEN_OLD /
2);
+ else
+ reg_count = addr - (ACPI_GPE0_BLK_ADDRESS + ACPI_GPE0_BLK_LEN / 2);
s->gpe0_en[reg_count] = val;
/* If disable GPE bit right after generating SCI on it,
* need deassert the intr to avoid redundant intrs
@@ -459,7 +502,7 @@ static void gpe_save(QEMUFile* f, void* opaque)
GPEState *s = (GPEState*)opaque;
int i;
- for ( i = 0; i < ACPI_GPE0_BLK_LEN / 2; i++ ) {
+ for ( i = 0; i < ACPI_GPE0_BLK_LEN_OLD / 2; i++ ) {
qemu_put_8s(f, &s->gpe0_sts[i]);
qemu_put_8s(f, &s->gpe0_en[i]);
}
@@ -468,21 +511,43 @@ static void gpe_save(QEMUFile* f, void* opaque)
if ( s->sci_asserted ) {
PIIX4ACPI_LOG(PIIX4ACPI_LOG_INFO, "gpe_save with sci
asserted!\n");
}
+
+ qemu_put_8s(f, &s->use_old_ioport);
}
static int gpe_load(QEMUFile* f, void* opaque, int version_id)
{
GPEState *s = (GPEState*)opaque;
int i;
- if (version_id != 1)
+ if (version_id > 2)
return -EINVAL;
- for ( i = 0; i < ACPI_GPE0_BLK_LEN / 2; i++ ) {
+ for ( i = 0; i < ACPI_GPE0_BLK_LEN_OLD / 2; i++ ) {
qemu_get_8s(f, &s->gpe0_sts[i]);
qemu_get_8s(f, &s->gpe0_en[i]);
}
qemu_get_8s(f, &s->sci_asserted);
+
+ if (version_id <= 1) {
+ s->use_old_ioport = 1;
+ } else {
+ qemu_get_8s(f, &s->use_old_ioport);
+ }
+
+ if (s->use_old_ioport) {
+ isa_unassign_ioport(ACPI_GPE0_BLK_ADDRESS, ACPI_GPE0_BLK_LEN);
+
+ register_ioport_read(ACPI_GPE0_BLK_ADDRESS_OLD, ACPI_GPE0_BLK_LEN_OLD /
2,
+ 1, gpe_sts_read, s);
+ register_ioport_read(ACPI_GPE0_BLK_ADDRESS_OLD + ACPI_GPE0_BLK_LEN_OLD
/ 2,
+ ACPI_GPE0_BLK_LEN_OLD / 2, 1, gpe_en_read, s);
+ register_ioport_write(ACPI_GPE0_BLK_ADDRESS_OLD, ACPI_GPE0_BLK_LEN_OLD
/ 2,
+ 1, gpe_sts_write, s);
+ register_ioport_write(ACPI_GPE0_BLK_ADDRESS_OLD + ACPI_GPE0_BLK_LEN_OLD
/ 2,
+ ACPI_GPE0_BLK_LEN_OLD / 2, 1, gpe_en_write, s);
+ }
+
return 0;
}
@@ -551,7 +616,7 @@ static void gpe_acpi_init(void)
gpe_en_write,
s);
- register_savevm("gpe", 0, 1, gpe_save, gpe_load, s);
+ register_savevm("gpe", 0, 2, gpe_save, gpe_load, s);
}
#ifdef CONFIG_PASSTHROUGH
@@ -703,7 +768,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t
smb_io_base,
pci_conf[0x43] = 0x00;
d->pm1_control = SCI_EN;
- acpi_map((PCIDevice *)d, 0, 0x1f40, 0x10, PCI_ADDRESS_SPACE_IO);
+ acpi_map((PCIDevice *)d, 0, ACPI_PM1A_EVT_BLK_ADDRESS, 0x10,
PCI_ADDRESS_SPACE_IO);
gpe_acpi_init();
#ifdef CONFIG_PASSTHROUGH
@@ -711,7 +776,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t
smb_io_base,
#endif
register_ioport_write(ACPI_DBG_IO_ADDR, 4, 4, acpi_dbg_writel, d);
- register_savevm("piix4acpi", 0, 1, piix4acpi_save,
piix4acpi_load, d);
+ register_savevm("piix4acpi", 0, 2, piix4acpi_save,
piix4acpi_load, d);
return NULL;
}
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2010-Oct-29 12:25 UTC
Re: [Xen-devel] [PATCH V3 3/6] xen, Intruduce pmtimer_change_ioport and HVM_PARAM_ACPI_NEW_IOPORT.
On 29/10/2010 13:08, "anthony.perard@citrix.com" <anthony.perard@citrix.com> wrote:> From: Anthony PERARD <anthony.perard@citrix.com> > > By default, Xen will handle the old ACPI IO port. But it can switch to > the new one by setting the HVM_PARAM_ACPI_NEW_IOPORT to 1.Fine, but this new parameter deserves a better explanatory comment in include/public/hvm/params.h. Its meaning is subtle and not immediately obvious. So go into some detail -- that it is basically a version number, current valid versions are 0 and 1, and the effect of setting each of those valid version numbers. Note that some other parameters have maybe half a page of accompanying explanatory comment. It''s better to write a bit too much rather than too little, and ensures our interface is well documented and hence well used and maintained, because others will understand it. Apart from this one point, I am happy for the entire patch series to be checked in. So once you''ve made that improvement: Acked-by: Keir Fraser <keir@xen.org> Thanks, Keir> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > xen/arch/x86/hvm/hvm.c | 9 +++++++++ > xen/arch/x86/hvm/pmtimer.c | 26 ++++++++++++++++++++++++-- > xen/include/asm-x86/hvm/vpt.h | 1 + > xen/include/public/hvm/params.h | 5 ++++- > 4 files changed, 38 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 94190d3..ea296e5 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -2991,6 +2991,15 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) > arg) > rc = -EINVAL; > > break; > + case HVM_PARAM_ACPI_NEW_IOPORT: > + if (a.value == 1) > + pmtimer_change_ioport(d, 1); > + else if (a.value == 0) > + pmtimer_change_ioport(d, 0); > + else > + rc = -EINVAL; > + > + break; > } > > if ( rc == 0 ) > diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c > index 1b9ab7b..f046201 100644 > --- a/xen/arch/x86/hvm/pmtimer.c > +++ b/xen/arch/x86/hvm/pmtimer.c > @@ -24,6 +24,9 @@ > #include <asm/acpi.h> /* for hvm_acpi_power_button prototype */ > > /* Slightly more readable port I/O addresses for the registers we intercept > */ > +#define PM1a_STS_ADDR_OLD (ACPI_PM1A_EVT_BLK_ADDRESS_OLD) > +#define PM1a_EN_ADDR_OLD (ACPI_PM1A_EVT_BLK_ADDRESS_OLD + 2) > +#define TMR_VAL_ADDR_OLD (ACPI_PM_TMR_BLK_ADDRESS_OLD) > #define PM1a_STS_ADDR (ACPI_PM1A_EVT_BLK_ADDRESS) > #define PM1a_EN_ADDR (ACPI_PM1A_EVT_BLK_ADDRESS + 2) > #define TMR_VAL_ADDR (ACPI_PM_TMR_BLK_ADDRESS) > @@ -155,16 +158,20 @@ static int handle_evt_io( > switch ( addr ) > { > /* PM1a_STS register bits are write-to-clear */ > + case PM1a_STS_ADDR_OLD: > case PM1a_STS_ADDR: > s->pm.pm1a_sts &= ~byte; > break; > + case PM1a_STS_ADDR_OLD + 1: > case PM1a_STS_ADDR + 1: > s->pm.pm1a_sts &= ~(byte << 8); > break; > > + case PM1a_EN_ADDR_OLD: > case PM1a_EN_ADDR: > s->pm.pm1a_en = (s->pm.pm1a_en & 0xff00) | byte; > break; > + case PM1a_EN_ADDR_OLD + 1: > case PM1a_EN_ADDR + 1: > s->pm.pm1a_en = (s->pm.pm1a_en & 0xff) | (byte << 8); > break; > @@ -272,6 +279,21 @@ static int pmtimer_load(struct domain *d, > hvm_domain_context_t *h) > HVM_REGISTER_SAVE_RESTORE(PMTIMER, pmtimer_save, pmtimer_load, > 1, HVMSR_PER_DOM); > > +void pmtimer_change_ioport(struct domain *d, int use_new) > +{ > + if (use_new) { > + register_portio_handler(d, TMR_VAL_ADDR, 4, handle_pmt_io); > + register_portio_handler(d, PM1a_STS_ADDR, 4, handle_evt_io); > + unregister_portio_handler(d, TMR_VAL_ADDR_OLD, 4); > + unregister_portio_handler(d, PM1a_STS_ADDR_OLD, 4); > + } else { > + register_portio_handler(d, TMR_VAL_ADDR_OLD, 4, handle_pmt_io); > + register_portio_handler(d, PM1a_STS_ADDR_OLD, 4, handle_evt_io); > + unregister_portio_handler(d, TMR_VAL_ADDR, 4); > + unregister_portio_handler(d, PM1a_STS_ADDR, 4); > + } > +} > + > void pmtimer_init(struct vcpu *v) > { > PMTState *s = &v->domain->arch.hvm_domain.pl_time.vpmt; > @@ -284,8 +306,8 @@ void pmtimer_init(struct vcpu *v) > > /* Intercept port I/O (need two handlers because PM1a_CNT is between > * PM1a_EN and TMR_VAL and is handled by qemu) */ > - register_portio_handler(v->domain, TMR_VAL_ADDR, 4, handle_pmt_io); > - register_portio_handler(v->domain, PM1a_STS_ADDR, 4, handle_evt_io); > + register_portio_handler(v->domain, TMR_VAL_ADDR_OLD, 4, handle_pmt_io); > + register_portio_handler(v->domain, PM1a_STS_ADDR_OLD, 4, handle_evt_io); > > /* Set up callback to fire SCIs when the MSB of TMR_VAL changes */ > init_timer(&s->timer, pmt_timer_callback, s, v->processor); > diff --git a/xen/include/asm-x86/hvm/vpt.h b/xen/include/asm-x86/hvm/vpt.h > index 65b0dff..6b68888 100644 > --- a/xen/include/asm-x86/hvm/vpt.h > +++ b/xen/include/asm-x86/hvm/vpt.h > @@ -179,6 +179,7 @@ void rtc_update_clock(struct domain *d); > void pmtimer_init(struct vcpu *v); > void pmtimer_deinit(struct domain *d); > void pmtimer_reset(struct domain *d); > +void pmtimer_change_ioport(struct domain *d, int use_new); > > void hpet_init(struct vcpu *v); > void hpet_deinit(struct domain *d); > diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h > index 673148b..40af8d8 100644 > --- a/xen/include/public/hvm/params.h > +++ b/xen/include/public/hvm/params.h > @@ -113,6 +113,9 @@ > #define HVM_PARAM_CONSOLE_PFN 17 > #define HVM_PARAM_CONSOLE_EVTCHN 18 > > -#define HVM_NR_PARAMS 19 > +/* to specify which firmware acpi ioport is used */ > +#define HVM_PARAM_ACPI_NEW_IOPORT 19 > + > +#define HVM_NR_PARAMS 20 > > #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2010-Oct-29 13:09 UTC
Re: [Xen-devel] [PATCH V3 3/6] xen, Intruduce pmtimer_change_ioport and HVM_PARAM_ACPI_NEW_IOPORT.
On Fri, 29 Oct 2010, Keir Fraser wrote:> On 29/10/2010 13:08, "anthony.perard@citrix.com" <anthony.perard@citrix.com> wrote: > > By default, Xen will handle the old ACPI IO port. But it can switch to > > the new one by setting the HVM_PARAM_ACPI_NEW_IOPORT to 1. > > Fine, but this new parameter deserves a better explanatory comment in > include/public/hvm/params.h. Its meaning is subtle and not immediately > obvious. So go into some detail -- that it is basically a version number, > current valid versions are 0 and 1, and the effect of setting each of those > valid version numbers. Note that some other parameters have maybe half a > page of accompanying explanatory comment. It''s better to write a bit too > much rather than too little, and ensures our interface is well documented > and hence well used and maintained, because others will understand it. > > Apart from this one point, I am happy for the entire patch series to be > checked in. So once you''ve made that improvement: > Acked-by: Keir Fraser <keir@xen.org>Thanks, I will do that! -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Pasi Kärkkäinen
2010-Oct-29 13:11 UTC
Re: [Xen-devel] [PATCH V3 0/6] firmware changes as part of QEMU/Xen merge.
On Fri, Oct 29, 2010 at 01:08:33PM +0100, anthony.perard@citrix.com wrote:> From: Anthony PERARD <anthony.perard@citrix.com> > > This patches are for Xen-unstable and Qemu-DM. > > With the integration of Xen into QEMU, we would like to minimise the change > and use the ACPI implementation of QEMU. So there are some change to make on > the firmware to match the QEMU''s BIOS. Some IO Ports are different and the > sleep state values are different. > > Change with the v2->v3: > > Xen will register the old ioport by default and switch to the new ioport if the > HVM_PARAM_ACPI_NEW_IOPORT is set. So, this series come with a new function, > unregister_io_handler, and a new HVM_PARAM. >Should it be called something else than HVM_PARAM_ACPI_NEW_IOPORT ? After a couple of years "NEW" might not make much sense anymore.. IOPORT_V2 ? Or something.. -- Pasi> > Change v1->v2: > > This time, both old and new ioport are handled. > In QEMU-Xen, the choice between one or the other is made when a saved state is > restored. > In Xen, both ioports are registered. > > > Anthony PERARD (5): > firmware, Change ACPI IO addresses and values to match QEMU BIOS. > xen, Introduce unregister_io_handler > xen, Intruduce pmtimer_change_ioport and HVM_PARAM_ACPI_NEW_IOPORT. > firmware, Set HVM_PARAM_ACPI_NEW_IOPORT > libxc, save/restore, Save the HVM_PARAM_ACPI_NEW_IOPORT > > tools/firmware/hvmloader/acpi/dsdt.asl | 12 ++++++------ > tools/firmware/hvmloader/hvmloader.c | 11 +++++++++++ > tools/libxc/xc_domain_restore.c | 12 ++++++++++++ > tools/libxc/xc_domain_save.c | 11 +++++++++++ > tools/libxc/xg_save_restore.h | 1 + > xen/arch/x86/hvm/hvm.c | 9 +++++++++ > xen/arch/x86/hvm/intercept.c | 26 ++++++++++++++++++++++++++ > xen/arch/x86/hvm/pmtimer.c | 26 ++++++++++++++++++++++++-- > xen/include/asm-x86/hvm/io.h | 8 ++++++++ > xen/include/asm-x86/hvm/vpt.h | 1 + > xen/include/public/hvm/ioreq.h | 16 +++++++++++++--- > xen/include/public/hvm/params.h | 5 ++++- > 12 files changed, 126 insertions(+), 12 deletions(-) > > > QEMU-Xen change: > > Anthony PERARD (1): > piix4acpi: change in ACPI to match the change in the BIOS. > > hw/piix4acpi.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++--------- > 1 files changed, 82 insertions(+), 17 deletions(-) > > -- > Anthony PERARD > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Oct-29 13:16 UTC
Re: [Xen-devel] [PATCH V3 0/6] firmware changes as part of QEMU/Xen merge.
On 29/10/2010 14:11, "Pasi Kärkkäinen" <pasik@iki.fi> wrote:>> Xen will register the old ioport by default and switch to the new ioport if >> the >> HVM_PARAM_ACPI_NEW_IOPORT is set. So, this series come with a new function, >> unregister_io_handler, and a new HVM_PARAM. >> > > Should it be called something else than HVM_PARAM_ACPI_NEW_IOPORT ? > After a couple of years "NEW" might not make much sense anymore.. > > IOPORT_V2 ? Or something..I''ve asked for a nice big explanatory comment to be added beside the parameter''s definition. Also we should treat this field as a version number, so baking a fixed version into its name is not a good idea. With the explanatory comment, I think its existing name is good enough. To do much better the name would have to become cumbersomely long. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Oct-29 13:18 UTC
Re: [Xen-devel] [PATCH V3 0/6] firmware changes as part of QEMU/Xen merge.
On 29/10/2010 14:16, "Keir Fraser" <keir@xen.org> wrote:>> Should it be called something else than HVM_PARAM_ACPI_NEW_IOPORT ? >> After a couple of years "NEW" might not make much sense anymore.. >> >> IOPORT_V2 ? Or something.. > > I''ve asked for a nice big explanatory comment to be added beside the > parameter''s definition. Also we should treat this field as a version number, > so baking a fixed version into its name is not a good idea. With the > explanatory comment, I think its existing name is good enough. To do much > better the name would have to become cumbersomely long.That said, HVM_PARAM_ACPI_IOPORTS_LOCATION might be better... I''m not personally that fussed either way however. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Pasi Kärkkäinen
2010-Oct-29 13:27 UTC
Re: [Xen-devel] [PATCH V3 0/6] firmware changes as part of QEMU/Xen merge.
On Fri, Oct 29, 2010 at 02:18:29PM +0100, Keir Fraser wrote:> On 29/10/2010 14:16, "Keir Fraser" <keir@xen.org> wrote: > > >> Should it be called something else than HVM_PARAM_ACPI_NEW_IOPORT ? > >> After a couple of years "NEW" might not make much sense anymore.. > >> > >> IOPORT_V2 ? Or something.. > > > > I''ve asked for a nice big explanatory comment to be added beside the > > parameter''s definition. Also we should treat this field as a version number, > > so baking a fixed version into its name is not a good idea. With the > > explanatory comment, I think its existing name is good enough. To do much > > better the name would have to become cumbersomely long. > > That said, HVM_PARAM_ACPI_IOPORTS_LOCATION might be better... I''m not > personally that fussed either way however. >Yeah, that sounds good.. -- Pasi _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2010-Oct-29 13:42 UTC
Re: [Xen-devel] [PATCH V3 0/6] firmware changes as part ofMU/Xen merge.
On Fri, 29 Oct 2010, Pasi Kärkkäinen wrote:> On Fri, Oct 29, 2010 at 02:18:29PM +0100, Keir Fraser wrote: > > On 29/10/2010 14:16, "Keir Fraser" <keir@xen.org> wrote: > > > > >> Should it be called something else than HVM_PARAM_ACPI_NEW_IOPORT ? > > >> After a couple of years "NEW" might not make much sense anymore.. > > >> > > >> IOPORT_V2 ? Or something.. > > > > > > I''ve asked for a nice big explanatory comment to be added beside the > > > parameter''s definition. Also we should treat this field as a version number, > > > so baking a fixed version into its name is not a good idea. With the > > > explanatory comment, I think its existing name is good enough. To do much > > > better the name would have to become cumbersomely long. > > > > That said, HVM_PARAM_ACPI_IOPORTS_LOCATION might be better... I''m not > > personally that fussed either way however. > > > > Yeah, that sounds good..This seam better, I will use this name with this value: - 0 : for the xen/old addresses. - 1 : for the qemu/new addresses. -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel