From mboxrd@z Thu Jan 1 00:00:00 1970 From: Beth Kon Subject: Re: [PATCH 2/4] Userspace changes for configuring irq0->inti2override (v3) Date: Tue, 12 May 2009 09:29:53 -0400 Message-ID: <4A0979D1.5020006@us.ibm.com> References: <1242062986-29383-1-git-send-email-eak@us.ibm.com> <1242062986-29383-2-git-send-email-eak@us.ibm.com> <20090512095345.GB19446@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: avi@redhat.com, kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from e31.co.us.ibm.com ([32.97.110.149]:43306 "EHLO e31.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752228AbZELN3I (ORCPT ); Tue, 12 May 2009 09:29:08 -0400 Received: from d03relay04.boulder.ibm.com (d03relay04.boulder.ibm.com [9.17.195.106]) by e31.co.us.ibm.com (8.13.1/8.13.1) with ESMTP id n4CDPNFT008270 for ; Tue, 12 May 2009 07:25:23 -0600 Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay04.boulder.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n4CDT7XR146098 for ; Tue, 12 May 2009 07:29:07 -0600 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n4CDT7io013054 for ; Tue, 12 May 2009 07:29:07 -0600 In-Reply-To: <20090512095345.GB19446@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Gleb Natapov wrote: > On Mon, May 11, 2009 at 01:29:44PM -0400, Beth Kon wrote: > >> Signed-off-by: Beth Kon >> >> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c >> index e1b19d7..bb74f38 100644 >> --- a/hw/fw_cfg.c >> +++ b/hw/fw_cfg.c >> @@ -279,6 +279,7 @@ void *fw_cfg_init(uint32_t ctl_port, uint32_t data_port, >> fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16); >> fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)nographic); >> fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus); >> + fw_cfg_add_i16(s, FW_CFG_IRQ0_OVERRIDE, (uint16_t)irq0override); >> >> > It is read as 1 byte by the BIOS, but it is 2 bytes here. And arch > specific config should be registered in arch specific place (hw/pc.c) > ok. > >> register_savevm("fw_cfg", -1, 1, fw_cfg_save, fw_cfg_load, s); >> qemu_register_reset(fw_cfg_reset, s); >> diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h >> index f616ed2..1de7360 100644 >> --- a/hw/fw_cfg.h >> +++ b/hw/fw_cfg.h >> @@ -19,6 +19,7 @@ >> >> #define FW_CFG_WRITE_CHANNEL 0x4000 >> #define FW_CFG_ARCH_LOCAL 0x8000 >> +#define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2) >> > This should go to hw/pc.c > ok. > >> #define FW_CFG_ENTRY_MASK ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL) >> >> #define FW_CFG_INVALID 0xffff >> diff --git a/hw/ioapic.c b/hw/ioapic.c >> index 0b70cf6..2d77a2c 100644 >> --- a/hw/ioapic.c >> +++ b/hw/ioapic.c >> @@ -23,6 +23,7 @@ >> >> #include "hw.h" >> #include "pc.h" >> +#include "sysemu.h" >> #include "qemu-timer.h" >> #include "host-utils.h" >> >> @@ -95,14 +96,13 @@ void ioapic_set_irq(void *opaque, int vector, int level) >> { >> IOAPICState *s = opaque; >> >> -#if 0 >> /* ISA IRQs map to GSI 1-1 except for IRQ0 which maps >> * to GSI 2. GSI maps to ioapic 1-1. This is not >> * the cleanest way of doing it but it should work. */ >> >> - if (vector == 0) >> + if (vector == 0 && irq0override) { >> vector = 2; >> -#endif >> + } >> >> if (vector >= 0 && vector < IOAPIC_NUM_PINS) { >> uint32_t mask = 1 << vector; >> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c >> index 8cb6faa..2e52c87 100644 >> --- a/qemu-kvm-x86.c >> +++ b/qemu-kvm-x86.c >> @@ -879,7 +879,11 @@ int kvm_arch_init_irq_routing(void) >> return r; >> } >> for (i = 0; i < 24; ++i) { >> - r = kvm_add_irq_route(kvm_context, i, KVM_IRQCHIP_IOAPIC, i); >> + if (i == 0) { >> + r = kvm_add_irq_route(kvm_context, i, KVM_IRQCHIP_IOAPIC, 2); >> + } else if (i != 2) { >> + r = kvm_add_irq_route(kvm_context, i, KVM_IRQCHIP_IOAPIC, i); >> + } >> > There is no entry for IRQ2, is this OK? What happens if IRQ2 triggers? > > Answered in separate email. >> if (r < 0) >> return r; >> } >> diff --git a/qemu-kvm.h b/qemu-kvm.h >> index dd045dd..6a1968a 100644 >> --- a/qemu-kvm.h >> +++ b/qemu-kvm.h >> @@ -165,6 +165,7 @@ void qemu_kvm_cpu_stop(CPUState *env); >> #define kvm_enabled() (kvm_allowed) >> #define qemu_kvm_irqchip_in_kernel() kvm_irqchip_in_kernel(kvm_context) >> #define qemu_kvm_pit_in_kernel() kvm_pit_in_kernel(kvm_context) >> +#define qemu_kvm_has_gsi_routing() kvm_has_gsi_routing(kvm_context) >> #define kvm_has_sync_mmu() qemu_kvm_has_sync_mmu() >> void kvm_init_vcpu(CPUState *env); >> void kvm_load_tsc(CPUState *env); >> @@ -173,6 +174,7 @@ void kvm_load_tsc(CPUState *env); >> #define kvm_nested 0 >> #define qemu_kvm_irqchip_in_kernel() (0) >> #define qemu_kvm_pit_in_kernel() (0) >> +#define qemu_kvm_has_gsi_routing() (0) >> #define kvm_has_sync_mmu() (0) >> #define kvm_load_registers(env) do {} while(0) >> #define kvm_save_registers(env) do {} while(0) >> diff --git a/sysemu.h b/sysemu.h >> index 1f45fd6..292bbc3 100644 >> --- a/sysemu.h >> +++ b/sysemu.h >> @@ -93,6 +93,7 @@ extern int graphic_width; >> extern int graphic_height; >> extern int graphic_depth; >> extern int nographic; >> +extern int irq0override; >> extern const char *keyboard_layout; >> extern int win2k_install_hack; >> extern int rtc_td_hack; >> diff --git a/vl.c b/vl.c >> index d9f0607..0bffc82 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -207,6 +207,7 @@ static int vga_ram_size; >> enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB; >> static DisplayState *display_state; >> int nographic; >> +int irq0override; >> static int curses; >> static int sdl; >> const char* keyboard_layout = NULL; >> @@ -5035,6 +5036,7 @@ int main(int argc, char **argv, char **envp) >> vga_ram_size = VGA_RAM_SIZE; >> snapshot = 0; >> nographic = 0; >> + irq0override = 1; >> > Why not do that when defining the variable? Yeah I realize this is how > it is done for other variables too, but why? > > Good question. I don't think there is any good reason. I was conforming to the existing style. >> curses = 0; >> kernel_filename = NULL; >> kernel_cmdline = ""; >> @@ -6129,8 +6131,14 @@ int main(int argc, char **argv, char **envp) >> } >> } >> >> - if (kvm_enabled()) >> - kvm_init_ap(); >> + if (kvm_enabled()) { >> + kvm_init_ap(); >> +#ifdef USE_KVM >> + if (kvm_irqchip && !qemu_kvm_has_gsi_routing()) { >> + irq0override = 0; >> + } >> +#endif >> + } >> >> machine->init(ram_size, vga_ram_size, boot_devices, >> kernel_filename, kernel_cmdline, initrd_filename, cpu_model); >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- > Gleb. >