[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/4] apic: add support for x2APIC mode
From: |
Igor Mammedov |
Subject: |
Re: [PATCH 1/4] apic: add support for x2APIC mode |
Date: |
Fri, 24 Feb 2023 15:29:32 +0100 |
On Tue, 21 Feb 2023 23:04:57 +0700
Bui Quang Minh <minhquangbui99@gmail.com> wrote:
> This commit refactors APIC registers read/write function to support both
> MMIO read/write in xAPIC mode and MSR read/write in x2APIC mode. Also,
> support larger APIC ID, self IPI, new IPI destination determination in
> x2APIC mode.
>
> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> ---
> hw/intc/apic.c | 211 +++++++++++++++++++++++++-------
> hw/intc/apic_common.c | 2 +-
> include/hw/i386/apic.h | 5 +-
> include/hw/i386/apic_internal.h | 2 +-
> 4 files changed, 172 insertions(+), 48 deletions(-)
>
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 2d3e55f4e2..205d5923ec 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -30,6 +30,7 @@
> #include "hw/i386/apic-msidef.h"
> #include "qapi/error.h"
> #include "qom/object.h"
> +#include "tcg/helper-tcg.h"
>
> #define MAX_APICS 255
I'm curious how does it work without increasing ^^^?
> #define MAX_APIC_WORDS 8
> @@ -48,7 +49,7 @@ DECLARE_INSTANCE_CHECKER(APICCommonState, APIC,
> static void apic_set_irq(APICCommonState *s, int vector_num, int
> trigger_mode);
> static void apic_update_irq(APICCommonState *s);
> static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
> - uint8_t dest, uint8_t dest_mode);
> + uint32_t dest, uint8_t dest_mode);
>
> /* Find first bit starting from msb */
> static int apic_fls_bit(uint32_t value)
> @@ -275,7 +276,7 @@ static void apic_bus_deliver(const uint32_t
> *deliver_bitmask,
> apic_set_irq(apic_iter, vector_num, trigger_mode) );
> }
>
> -void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
> +void apic_deliver_irq(uint32_t dest, uint8_t dest_mode, uint8_t
> delivery_mode,
> uint8_t vector_num, uint8_t trigger_mode)
> {
> uint32_t deliver_bitmask[MAX_APIC_WORDS];
> @@ -287,8 +288,38 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
> uint8_t delivery_mode,
> apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num,
> trigger_mode);
> }
>
> +bool is_x2apic_mode(DeviceState *dev)
> +{
> + APICCommonState *s = APIC(dev);
> +
> + return s->apicbase & MSR_IA32_APICBASE_EXTD;
> +}
> +
> static void apic_set_base(APICCommonState *s, uint64_t val)
> {
> + /*
> + * Transition into invalid state
> + * (s->apicbase & MSR_IA32_APICBASE_ENABLE == 0) &&
> + * (s->apicbase & MSR_IA32_APICBASE_EXTD) == 1
> + */
> + if (!(val & MSR_IA32_APICBASE_ENABLE) &&
> + (val & MSR_IA32_APICBASE_EXTD))
> + raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
> +
> + /* Invalid transition from disabled mode to x2APIC */
> + if (!(s->apicbase & MSR_IA32_APICBASE_ENABLE) &&
> + !(s->apicbase & MSR_IA32_APICBASE_EXTD) &&
> + (val & MSR_IA32_APICBASE_ENABLE) &&
> + (val & MSR_IA32_APICBASE_EXTD))
> + raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
> +
> + /* Invalid transition from x2APIC to xAPIC */
> + if ((s->apicbase & MSR_IA32_APICBASE_ENABLE) &&
> + (s->apicbase & MSR_IA32_APICBASE_EXTD) &&
> + (val & MSR_IA32_APICBASE_ENABLE) &&
> + !(val & MSR_IA32_APICBASE_EXTD))
> + raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
> +
> s->apicbase = (val & 0xfffff000) |
> (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
> /* if disabled, cannot be enabled again */
> @@ -297,6 +328,21 @@ static void apic_set_base(APICCommonState *s, uint64_t
> val)
> cpu_clear_apic_feature(&s->cpu->env);
> s->spurious_vec &= ~APIC_SV_ENABLE;
> }
> +
> + /* Transition from xAPIC to x2APIC */
> + if (cpu_has_x2apic_feature(&s->cpu->env) &&
> + !(s->apicbase & MSR_IA32_APICBASE_EXTD) &&
> + (val & MSR_IA32_APICBASE_EXTD)) {
> + s->apicbase |= MSR_IA32_APICBASE_EXTD;
> +
> + s->log_dest = ((s->initial_apic_id & 0xffff0) << 16) |
> + (1 << (s->initial_apic_id & 0xf));
> +
> + /* disable MMIO interface */
> + qemu_mutex_lock_iothread();
> + memory_region_set_enabled(&s->io_memory, false);
> + qemu_mutex_unlock_iothread();
> + }
> }
>
> static void apic_set_tpr(APICCommonState *s, uint8_t val)
> @@ -454,7 +500,7 @@ static int apic_find_dest(uint8_t dest)
> }
>
> static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
> - uint8_t dest, uint8_t dest_mode)
> + uint32_t dest, uint8_t dest_mode)
> {
> APICCommonState *apic_iter;
> int i;
> @@ -474,14 +520,22 @@ static void apic_get_delivery_bitmask(uint32_t
> *deliver_bitmask,
> for(i = 0; i < MAX_APICS; i++) {
> apic_iter = local_apics[i];
> if (apic_iter) {
> - if (apic_iter->dest_mode == 0xf) {
> - if (dest & apic_iter->log_dest)
> - apic_set_bit(deliver_bitmask, i);
> - } else if (apic_iter->dest_mode == 0x0) {
> - if ((dest & 0xf0) == (apic_iter->log_dest & 0xf0) &&
> - (dest & apic_iter->log_dest & 0x0f)) {
> + /* x2APIC mode */
> + if (apic_iter->apicbase & MSR_IA32_APICBASE_EXTD) {
> + if ((dest & 0xffff0000) == (apic_iter->log_dest &
> 0xffff0000) &&
> + (dest & apic_iter->log_dest & 0xffff)) {
> apic_set_bit(deliver_bitmask, i);
> }
> + } else {
> + if (apic_iter->dest_mode == 0xf) {
> + if (dest & apic_iter->log_dest)
> + apic_set_bit(deliver_bitmask, i);
> + } else if (apic_iter->dest_mode == 0x0) {
> + if ((dest & 0xf0) == (apic_iter->log_dest & 0xf0) &&
> + (dest & apic_iter->log_dest & 0x0f)) {
> + apic_set_bit(deliver_bitmask, i);
> + }
> + }
> }
> } else {
> break;
> @@ -508,13 +562,12 @@ void apic_sipi(DeviceState *dev)
> s->wait_for_sipi = 0;
> }
>
> -static void apic_deliver(DeviceState *dev, uint8_t dest, uint8_t dest_mode,
> +static void apic_deliver(DeviceState *dev, uint32_t dest, uint8_t dest_mode,
> uint8_t delivery_mode, uint8_t vector_num,
> - uint8_t trigger_mode)
> + uint8_t trigger_mode, uint8_t dest_shorthand)
> {
> APICCommonState *s = APIC(dev);
> uint32_t deliver_bitmask[MAX_APIC_WORDS];
> - int dest_shorthand = (s->icr[0] >> 18) & 3;
> APICCommonState *apic_iter;
>
> switch (dest_shorthand) {
> @@ -635,16 +688,11 @@ static void apic_timer(void *opaque)
> apic_timer_update(s, s->next_time);
> }
>
> -static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
> +uint64_t apic_register_read(int index)
> {
> DeviceState *dev;
> APICCommonState *s;
> - uint32_t val;
> - int index;
> -
> - if (size < 4) {
> - return 0;
> - }
> + uint64_t val;
>
> dev = cpu_get_current_apic();
> if (!dev) {
> @@ -652,10 +700,12 @@ static uint64_t apic_mem_read(void *opaque, hwaddr
> addr, unsigned size)
> }
> s = APIC(dev);
>
> - index = (addr >> 4) & 0xff;
> switch(index) {
> case 0x02: /* id */
> - val = s->id << 24;
> + if (is_x2apic_mode(dev))
> + val = s->initial_apic_id;
> + else
> + val = s->id << 24;
> break;
> case 0x03: /* version */
> val = s->version | ((APIC_LVT_NB - 1) << 16);
> @@ -678,9 +728,16 @@ static uint64_t apic_mem_read(void *opaque, hwaddr addr,
> unsigned size)
> val = 0;
> break;
> case 0x0d:
> - val = s->log_dest << 24;
> + if (is_x2apic_mode(dev))
> + val = s->log_dest;
> + else
> + val = s->log_dest << 24;
> break;
> case 0x0e:
> + if (is_x2apic_mode(dev)) {
> + raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
> + }
> +
> val = (s->dest_mode << 28) | 0xfffffff;
> break;
> case 0x0f:
> @@ -719,6 +776,22 @@ static uint64_t apic_mem_read(void *opaque, hwaddr addr,
> unsigned size)
> val = 0;
> break;
> }
> +
> + return val;
> +}
> +
> +static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + uint32_t val;
> + int index;
> +
> + if (size < 4) {
> + return 0;
> + }
> +
> + index = (addr >> 4) & 0xff;
> + val = (uint32_t) apic_register_read(index);
> +
> trace_apic_mem_readl(addr, val);
> return val;
> }
> @@ -736,27 +809,10 @@ static void apic_send_msi(MSIMessage *msi)
> apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode);
> }
>
> -static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
> - unsigned size)
> +void apic_register_write(int index, uint64_t val)
> {
> DeviceState *dev;
> APICCommonState *s;
> - int index = (addr >> 4) & 0xff;
> -
> - if (size < 4) {
> - return;
> - }
> -
> - if (addr > 0xfff || !index) {
> - /* MSI and MMIO APIC are at the same memory location,
> - * but actually not on the global bus: MSI is on PCI bus
> - * APIC is connected directly to the CPU.
> - * Mapping them on the global bus happens to work because
> - * MSI registers are reserved in APIC MMIO and vice versa. */
> - MSIMessage msi = { .address = addr, .data = val };
> - apic_send_msi(&msi);
> - return;
> - }
>
> dev = cpu_get_current_apic();
> if (!dev) {
> @@ -764,10 +820,12 @@ static void apic_mem_write(void *opaque, hwaddr addr,
> uint64_t val,
> }
> s = APIC(dev);
>
> - trace_apic_mem_writel(addr, val);
> -
> switch(index) {
> case 0x02:
> + if (is_x2apic_mode(dev)) {
> + raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
> + }
> +
> s->id = (val >> 24);
> break;
> case 0x03:
> @@ -787,9 +845,17 @@ static void apic_mem_write(void *opaque, hwaddr addr,
> uint64_t val,
> apic_eoi(s);
> break;
> case 0x0d:
> + if (is_x2apic_mode(dev)) {
> + raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
> + }
> +
> s->log_dest = val >> 24;
> break;
> case 0x0e:
> + if (is_x2apic_mode(dev)) {
> + raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
> + }
> +
> s->dest_mode = val >> 28;
> break;
> case 0x0f:
> @@ -801,13 +867,27 @@ static void apic_mem_write(void *opaque, hwaddr addr,
> uint64_t val,
> case 0x20 ... 0x27:
> case 0x28:
> break;
> - case 0x30:
> + case 0x30: {
> + uint32_t dest;
> +
> s->icr[0] = val;
> - apic_deliver(dev, (s->icr[1] >> 24) & 0xff, (s->icr[0] >> 11) & 1,
> + if (is_x2apic_mode(dev)) {
> + s->icr[1] = val >> 32;
> + dest = s->icr[1];
> + } else {
> + dest = (s->icr[1] >> 24) & 0xff;
> + }
> +
> + apic_deliver(dev, dest, (s->icr[0] >> 11) & 1,
> (s->icr[0] >> 8) & 7, (s->icr[0] & 0xff),
> - (s->icr[0] >> 15) & 1);
> + (s->icr[0] >> 15) & 1, (s->icr[0] >> 18) & 3);
> break;
> + }
> case 0x31:
> + if (is_x2apic_mode(dev)) {
> + raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
> + }
> +
> s->icr[1] = val;
> break;
> case 0x32 ... 0x37:
> @@ -836,12 +916,53 @@ static void apic_mem_write(void *opaque, hwaddr addr,
> uint64_t val,
> s->count_shift = (v + 1) & 7;
> }
> break;
> + case 0x3f: {
> + int vector = val & 0xff;
> +
> + if (is_x2apic_mode(dev)) {
> + raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
> + }
> +
> + /*
> + * Self IPI is identical to IPI with
> + * - Destination shorthand: 1 (Self)
> + * - Trigger mode: 0 (Edge)
> + * - Delivery mode: 0 (Fixed)
> + */
> + apic_deliver(dev, 0, 0, APIC_DM_FIXED, vector, 0, 1);
> +
> + break;
> + }
> default:
> s->esr |= APIC_ESR_ILLEGAL_ADDRESS;
> break;
> }
> }
>
> +static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
> + unsigned size)
> +{
> + int index = (addr >> 4) & 0xff;
> +
> + if (size < 4) {
> + return;
> + }
> +
> + if (addr > 0xfff || !index) {
> + /* MSI and MMIO APIC are at the same memory location,
> + * but actually not on the global bus: MSI is on PCI bus
> + * APIC is connected directly to the CPU.
> + * Mapping them on the global bus happens to work because
> + * MSI registers are reserved in APIC MMIO and vice versa. */
> + MSIMessage msi = { .address = addr, .data = val };
> + apic_send_msi(&msi);
> + return;
> + }
> +
> + trace_apic_mem_writel(addr, val);
> + apic_register_write(index, val);
> +}
> +
> static void apic_pre_save(APICCommonState *s)
> {
> apic_sync_vapic(s, SYNC_FROM_VAPIC);
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 4a34f03047..9ea1397530 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -366,7 +366,7 @@ static const VMStateDescription vmstate_apic_common = {
> VMSTATE_UINT8(arb_id, APICCommonState),
> VMSTATE_UINT8(tpr, APICCommonState),
> VMSTATE_UINT32(spurious_vec, APICCommonState),
> - VMSTATE_UINT8(log_dest, APICCommonState),
> + VMSTATE_UINT32(log_dest, APICCommonState),
> VMSTATE_UINT8(dest_mode, APICCommonState),
> VMSTATE_UINT32_ARRAY(isr, APICCommonState, 8),
> VMSTATE_UINT32_ARRAY(tmr, APICCommonState, 8),
> diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
> index bdc15a7a73..871ca94c5c 100644
> --- a/include/hw/i386/apic.h
> +++ b/include/hw/i386/apic.h
> @@ -3,7 +3,7 @@
>
>
> /* apic.c */
> -void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
> +void apic_deliver_irq(uint32_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);
> void apic_deliver_pic_intr(DeviceState *s, int level);
> @@ -18,6 +18,9 @@ void apic_sipi(DeviceState *s);
> void apic_poll_irq(DeviceState *d);
> void apic_designate_bsp(DeviceState *d, bool bsp);
> int apic_get_highest_priority_irr(DeviceState *dev);
> +uint64_t apic_register_read(int index);
> +void apic_register_write(int index, uint64_t val);
> +bool is_x2apic_mode(DeviceState *d);
>
> /* pc.c */
> DeviceState *cpu_get_current_apic(void);
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index 5f2ba24bfc..5f60cd5e78 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -164,7 +164,7 @@ struct APICCommonState {
> uint8_t arb_id;
> uint8_t tpr;
> uint32_t spurious_vec;
> - uint8_t log_dest;
> + uint32_t log_dest;
> uint8_t dest_mode;
> uint32_t isr[8]; /* in service register */
> uint32_t tmr[8]; /* trigger mode register */
[PATCH 2/4] i386/tcg: implement x2APIC registers MSR access, Bui Quang Minh, 2023/02/21
[PATCH 4/4] test/avocado: test Linux boot up in x2APIC with userspace local APIC, Bui Quang Minh, 2023/02/21
[PATCH 3/4] intel_iommu: allow Extended Interrupt Mode when using userspace local APIC, Bui Quang Minh, 2023/02/21