Marc Morcos <[email protected]> writes:
> APICBASE is 36-bits wide, so this commit resizes it to hold the full data.
>
> Signed-off-by: Marc Morcos <[email protected]>
> ---
> hw/intc/apic_common.c | 4 ++--
> include/hw/i386/apic_internal.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index ec9e978b0b..1e9aba2e48 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -233,7 +233,7 @@ static void apic_reset_common(DeviceState *dev)
> {
> APICCommonState *s = APIC_COMMON(dev);
> APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
> - uint32_t bsp;
> + uint64_t bsp;
>
> bsp = s->apicbase & MSR_IA32_APICBASE_BSP;
This seems overkill for something considering MSR_IA32_APICBASE_BSP is a
single bit (1<<8) and the reset never overflows as APIC_DEFAULT_ADDRESS
is within the 32 bit range.
> s->apicbase = APIC_DEFAULT_ADDRESS | bsp | MSR_IA32_APICBASE_ENABLE;
> @@ -363,7 +363,7 @@ static const VMStateDescription vmstate_apic_common = {
> .post_load = apic_dispatch_post_load,
> .priority = MIG_PRI_APIC,
> .fields = (const VMStateField[]) {
> - VMSTATE_UINT32(apicbase, APICCommonState),
> + VMSTATE_UINT64(apicbase, APICCommonState),
Changing this is problematic as you now have to deal with migration
between older and current QEMU's.
> VMSTATE_UINT8(id, APICCommonState),
> VMSTATE_UINT8(arb_id, APICCommonState),
> VMSTATE_UINT8(tpr, APICCommonState),
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index 4a62fdceb4..c7ee65ce1d 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -158,7 +158,7 @@ struct APICCommonState {
>
> MemoryRegion io_memory;
> X86CPU *cpu;
> - uint32_t apicbase;
> + uint64_t apicbase;
> uint8_t id; /* legacy APIC ID */
> uint32_t initial_apic_id;
> uint8_t version;
I'll defer to the x86 experts here but perhaps an alternative would be
to clamp kvm_apic_set_base() which seems to be the only place where you
can set it and not get clamped like in apic_set_base()?
--
Alex Bennée
Virtualisation Tech Lead @ Linaro