| Message ID | 20140430191816.GA16023@ERROL.INI.CMU.EDU |
|---|---|
| State | New |
| Headers | show |
Il 30/04/2014 21:18, Gabriel L. Somlo ha scritto: > Some guests (e.g. 0S X) more or less arbitrarily insist on a > minimum lapic version of 0x14 in order to successfully boot. > This patch bumps the emulated apic version on piix and q35 > machine types >= 2.1. > > Signed-off-by: Gabriel L. Somlo <somlo@cmu.edu> > --- > > On Thu, Mar 06, 2014 at 09:50:48AM +0200, Michael S. Tsirkin wrote: >> On Sat, Mar 01, 2014 at 07:17:07PM -0500, Gabriel L. Somlo wrote: >>> On Sat, Mar 01, 2014 at 03:46:22PM +0100, Paolo Bonzini wrote: >>>> >>>> Looks good, but you have to make this a qdev property so that older >>>> versions keep using version 0x11. >> >> [...] >> >> No, your patch is fine generally, it's not about users tweaking the >> version. >> It's about users getting compatible behaviour when they specify >> e.g. -M pc-1.6 >> >> Using property here is an implementation detail not exposed to users. >> Look for all the PC_COMPAT macros as an example of that. > > OK, in the mean time I figured out how PC_[Q35]_COMPAT_* works, > so here's another iteration, where we only bump the emulated lapic > version to 0x14 for the newest machine types (2.1 and up). > > Let me know what you think. > > Thanks, > Gabriel > > > hw/i386/pc_piix.c | 7 +++++++ > hw/i386/pc_q35.c | 7 +++++++ > hw/intc/apic.c | 4 +++- > include/hw/i386/apic.h | 2 ++ > 4 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 5b3594b..a488ec9 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -29,6 +29,7 @@ > #include "hw/i386/pc.h" > #include "hw/i386/apic.h" > #include "hw/i386/smbios.h" > +#include "hw/i386/apic.h" > #include "hw/pci/pci.h" > #include "hw/pci/pci_ids.h" > #include "hw/usb.h" > @@ -62,6 +63,7 @@ static bool has_pci_info; > static bool has_acpi_build = true; > static bool smbios_defaults = true; > static bool smbios_legacy_mode; > +static bool soft_apic_v14 = true; > /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to > * host addresses aligned at 1Gbyte boundaries. This way we can use 1GByte > * pages in the host. > @@ -102,6 +104,10 @@ static void pc_init1(QEMUMachineInitArgs *args, > exit(1); > } > > + if (soft_apic_v14) { > + apic_version = 0x14; > + } > + > icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); > object_property_add_child(qdev_get_machine(), "icc-bridge", > OBJECT(icc_bridge), NULL); > @@ -266,6 +272,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args) > static void pc_compat_2_0(QEMUMachineInitArgs *args) > { > smbios_legacy_mode = true; > + soft_apic_v14 = false; > } > > static void pc_compat_1_7(QEMUMachineInitArgs *args) > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 5b48231..3779b48 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -40,6 +40,7 @@ > #include "exec/address-spaces.h" > #include "hw/i386/ich9.h" > #include "hw/i386/smbios.h" > +#include "hw/i386/apic.h" > #include "hw/ide/pci.h" > #include "hw/ide/ahci.h" > #include "hw/usb.h" > @@ -52,6 +53,7 @@ static bool has_pci_info; > static bool has_acpi_build = true; > static bool smbios_defaults = true; > static bool smbios_legacy_mode; > +static bool soft_apic_v14 = true; > /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to > * host addresses aligned at 1Gbyte boundaries. This way we can use 1GByte > * pages in the host. > @@ -89,6 +91,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args) > exit(1); > } > > + if (soft_apic_v14) { > + apic_version = 0x14; > + } > + > icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); > object_property_add_child(qdev_get_machine(), "icc-bridge", > OBJECT(icc_bridge), NULL); > @@ -244,6 +250,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) > static void pc_compat_2_0(QEMUMachineInitArgs *args) > { > smbios_legacy_mode = true; > + soft_apic_v14 = false; > } > > static void pc_compat_1_7(QEMUMachineInitArgs *args) > diff --git a/hw/intc/apic.c b/hw/intc/apic.c > index 2f40cba..b53e8ff 100644 > --- a/hw/intc/apic.c > +++ b/hw/intc/apic.c > @@ -32,6 +32,8 @@ > #define SYNC_TO_VAPIC 0x2 > #define SYNC_ISR_IRR_TO_VAPIC 0x4 > > +uint8_t apic_version = 0x11; > + > static APICCommonState *local_apics[MAX_APICS + 1]; > > static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode); > @@ -675,7 +677,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr) > val = s->id << 24; > break; > case 0x03: /* version */ > - val = 0x11 | ((APIC_LVT_NB - 1) << 16); /* version 0x11 */ > + val = apic_version | ((APIC_LVT_NB - 1) << 16); I think it's preferrable to have apic_version be a device property with a default of 0x14, and use the compat_props to make it 0x11 on older machine types. Paolo > break; > case 0x08: > apic_sync_vapic(s, SYNC_FROM_VAPIC); > diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h > index 1d48e02..4e79240 100644 > --- a/include/hw/i386/apic.h > +++ b/include/hw/i386/apic.h > @@ -4,6 +4,8 @@ > #include "qemu-common.h" > > /* apic.c */ > +extern uint8_t apic_version; > + > void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode, > uint8_t vector_num, uint8_t trigger_mode); > int apic_accept_pic_intr(DeviceState *s); >
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 5b3594b..a488ec9 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -29,6 +29,7 @@ #include "hw/i386/pc.h" #include "hw/i386/apic.h" #include "hw/i386/smbios.h" +#include "hw/i386/apic.h" #include "hw/pci/pci.h" #include "hw/pci/pci_ids.h" #include "hw/usb.h" @@ -62,6 +63,7 @@ static bool has_pci_info; static bool has_acpi_build = true; static bool smbios_defaults = true; static bool smbios_legacy_mode; +static bool soft_apic_v14 = true; /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to * host addresses aligned at 1Gbyte boundaries. This way we can use 1GByte * pages in the host. @@ -102,6 +104,10 @@ static void pc_init1(QEMUMachineInitArgs *args, exit(1); } + if (soft_apic_v14) { + apic_version = 0x14; + } + icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); object_property_add_child(qdev_get_machine(), "icc-bridge", OBJECT(icc_bridge), NULL); @@ -266,6 +272,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args) static void pc_compat_2_0(QEMUMachineInitArgs *args) { smbios_legacy_mode = true; + soft_apic_v14 = false; } static void pc_compat_1_7(QEMUMachineInitArgs *args) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 5b48231..3779b48 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -40,6 +40,7 @@ #include "exec/address-spaces.h" #include "hw/i386/ich9.h" #include "hw/i386/smbios.h" +#include "hw/i386/apic.h" #include "hw/ide/pci.h" #include "hw/ide/ahci.h" #include "hw/usb.h" @@ -52,6 +53,7 @@ static bool has_pci_info; static bool has_acpi_build = true; static bool smbios_defaults = true; static bool smbios_legacy_mode; +static bool soft_apic_v14 = true; /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to * host addresses aligned at 1Gbyte boundaries. This way we can use 1GByte * pages in the host. @@ -89,6 +91,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args) exit(1); } + if (soft_apic_v14) { + apic_version = 0x14; + } + icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); object_property_add_child(qdev_get_machine(), "icc-bridge", OBJECT(icc_bridge), NULL); @@ -244,6 +250,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) static void pc_compat_2_0(QEMUMachineInitArgs *args) { smbios_legacy_mode = true; + soft_apic_v14 = false; } static void pc_compat_1_7(QEMUMachineInitArgs *args) diff --git a/hw/intc/apic.c b/hw/intc/apic.c index 2f40cba..b53e8ff 100644 --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -32,6 +32,8 @@ #define SYNC_TO_VAPIC 0x2 #define SYNC_ISR_IRR_TO_VAPIC 0x4 +uint8_t apic_version = 0x11; + static APICCommonState *local_apics[MAX_APICS + 1]; static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode); @@ -675,7 +677,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr) val = s->id << 24; break; case 0x03: /* version */ - val = 0x11 | ((APIC_LVT_NB - 1) << 16); /* version 0x11 */ + val = apic_version | ((APIC_LVT_NB - 1) << 16); break; case 0x08: apic_sync_vapic(s, SYNC_FROM_VAPIC); diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h index 1d48e02..4e79240 100644 --- a/include/hw/i386/apic.h +++ b/include/hw/i386/apic.h @@ -4,6 +4,8 @@ #include "qemu-common.h" /* apic.c */ +extern uint8_t apic_version; + void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode, uint8_t vector_num, uint8_t trigger_mode); int apic_accept_pic_intr(DeviceState *s);