@@ -22,6 +22,6 @@ void apic_designate_bsp(DeviceState *d, bool bsp);
int apic_get_highest_priority_irr(DeviceState *dev);
/* pc.c */
-DeviceState *cpu_get_current_apic(void);
+DeviceState *cpu_get_current_apic(int cpu_index);
#endif
@@ -585,14 +585,11 @@ int cpu_get_pic_interrupt(CPUX86State *env)
return intno;
}
-DeviceState *cpu_get_current_apic(void)
+DeviceState *cpu_get_current_apic(int cpu_index)
{
- if (current_cpu) {
- X86CPU *cpu = X86_CPU(current_cpu);
- return cpu->apic_state;
- } else {
- return NULL;
- }
+ CPUState *cs = qemu_get_cpu(cpu_index);
+ X86CPU *cpu = X86_CPU(cs);
+ return cpu->apic_state;
}
void gsi_handler(void *opaque, int n, int level)
@@ -18,9 +18,11 @@
*/
#include "qemu/osdep.h"
#include "qemu/thread.h"
+#include "qemu/log.h"
#include "hw/i386/apic_internal.h"
#include "hw/i386/apic.h"
#include "hw/i386/ioapic.h"
+#include "hw/i386/ioapic_internal.h"
#include "hw/intc/i8259.h"
#include "hw/pci/msi.h"
#include "qemu/host-utils.h"
@@ -634,21 +636,23 @@ 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)
+static MemTxResult apic_mem_read(void *opaque, hwaddr addr, uint64_t *data,
+ unsigned int size, MemTxAttrs attrs)
{
DeviceState *dev;
APICCommonState *s;
uint32_t val;
int index;
- if (size < 4) {
- return 0;
+ if (attrs.requester_type != MTRT_CPU) {
+ return MEMTX_ACCESS_ERROR;
}
+ dev = cpu_get_current_apic(attrs.requester_id);
- dev = cpu_get_current_apic();
- if (!dev) {
- return 0;
+ if (size < 4) {
+ return MEMTX_ERROR;
}
+
s = APIC(dev);
index = (addr >> 4) & 0xff;
@@ -719,7 +723,8 @@ static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
break;
}
trace_apic_mem_readl(addr, val);
- return val;
+ *data = val;
+ return MEMTX_OK;
}
static void apic_send_msi(MSIMessage *msi)
@@ -735,32 +740,45 @@ 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)
+static MemTxResult apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned int size, MemTxAttrs attrs)
{
DeviceState *dev;
APICCommonState *s;
int index = (addr >> 4) & 0xff;
if (size < 4) {
- return;
+ return MEMTX_ERROR;
}
+ /*
+ * 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.
+ *
+ * We can check the MemTxAttrs to check they are coming from where
+ * we expect. Even though the MSI registers are reserved in APIC
+ * MMIO and vice versa they shouldn't respond to CPU writes.
+ */
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. */
+ switch (attrs.requester_type) {
+ case MTRT_MACHINE: /* MEMTX_IOPIC */
+ case MTRT_PCI: /* PCI signalled MSI */
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: rejecting write from %d",
+ __func__, attrs.requester_id);
+ return MEMTX_ACCESS_ERROR;
+ }
MSIMessage msi = { .address = addr, .data = val };
apic_send_msi(&msi);
- return;
+ return MEMTX_OK;
}
- dev = cpu_get_current_apic();
- if (!dev) {
- return;
+ if (attrs.requester_type != MTRT_CPU) {
+ return MEMTX_ACCESS_ERROR;
}
+ dev = cpu_get_current_apic(attrs.requester_id);
s = APIC(dev);
trace_apic_mem_writel(addr, val);
@@ -839,6 +857,8 @@ static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
s->esr |= APIC_ESR_ILLEGAL_ADDRESS;
break;
}
+
+ return MEMTX_OK;
}
static void apic_pre_save(APICCommonState *s)
@@ -856,8 +876,8 @@ static void apic_post_load(APICCommonState *s)
}
static const MemoryRegionOps apic_io_ops = {
- .read = apic_mem_read,
- .write = apic_mem_write,
+ .read_with_attrs = apic_mem_read,
+ .write_with_attrs = apic_mem_write,
.impl.min_access_size = 1,
.impl.max_access_size = 4,
.valid.min_access_size = 1,
This allows us to correctly model invalid accesses to the interrupt controller as well as avoiding the use of current_cpu hacks to find the APIC structure. We have to ensure we check for MSI signals first which shouldn't arrive from the CPU but are either triggered by PCI or internal IOAPIC writes. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Xu <peterx@redhat.com> --- v1 - don't validate requester_id for MTRT_MACHINE, just assume IOPIC --- include/hw/i386/apic.h | 2 +- hw/i386/x86.c | 11 +++----- hw/intc/apic.c | 62 ++++++++++++++++++++++++++++-------------- 3 files changed, 46 insertions(+), 29 deletions(-)