From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33009) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2elE-0001Fn-P2 for qemu-devel@nongnu.org; Tue, 17 May 2016 09:10:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b2el8-0000KT-K3 for qemu-devel@nongnu.org; Tue, 17 May 2016 09:10:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33527) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2el8-0000JU-Bo for qemu-devel@nongnu.org; Tue, 17 May 2016 09:10:02 -0400 References: <1462568028-31037-1-git-send-email-rkrcmar@redhat.com> <1462568028-31037-3-git-send-email-rkrcmar@redhat.com> <20160513173307.GP4457@thinpad.lan.raisama.net> From: Paolo Bonzini Message-ID: <573B181D.1000604@redhat.com> Date: Tue, 17 May 2016 15:09:49 +0200 MIME-Version: 1.0 In-Reply-To: <20160513173307.GP4457@thinpad.lan.raisama.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/4] intel_iommu: use deliver_msi APIC callback List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Cc: qemu-devel@nongnu.org, "Lan, Tianyu" , Igor Mammedov , Jan Kiszka , Peter Xu , Richard Henderson On 13/05/2016 19:33, Eduardo Habkost wrote: > On Fri, May 06, 2016 at 10:53:46PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: >> The memory-mapped interface cannot express x2APIC destinations that ar= e >> a result of remapping. >> >> Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 >> --- >> hw/i386/intel_iommu.c | 29 ++++++++++++++++++----------- >> 1 file changed, 18 insertions(+), 11 deletions(-) >> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> index bee85e469477..d10064289551 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -26,6 +26,7 @@ >> #include "hw/pci/pci.h" >> #include "hw/boards.h" >> #include "hw/i386/x86-iommu.h" >> +#include "hw/i386/apic_internal.h" >> =20 >> /*#define DEBUG_INTEL_IOMMU*/ >> #ifdef DEBUG_INTEL_IOMMU >> @@ -268,24 +269,33 @@ static void vtd_update_iotlb(IntelIOMMUState *s,= uint16_t source_id, >> g_hash_table_replace(s->iotlb, key, entry); >> } >> =20 >> +static void apic_deliver_msi(MSIMessage *msi) >> +{ >> + /* Conjure apic-bound msi delivery out of thin air. */ >> + X86CPU *cpu =3D X86_CPU(first_cpu); >> + APICCommonState *apic_state =3D APIC_COMMON(cpu->apic_state); >> + APICCommonClass *apic_class =3D APIC_COMMON_GET_CLASS(apic_state)= ; >=20 > I see a pattern here: >=20 > hw/i386/kvmvapic.c-static void do_vapic_enable(void *data) > hw/i386/kvmvapic.c-{ > [...] > hw/i386/kvmvapic.c- X86CPU *cpu =3D X86_CPU(first_cpu); > [...] > hw/i386/kvmvapic.c: apic_enable_vapic(cpu->apic_state, s->vapic_padd= r); > [...] Here first_cpu is just the CPU that do_vapic_enable is being called on (via run_on_cpu). So this one can use the posted patch that passes a CPUState* to run_on_cpu callbacks. > hw/i386/pc.c-static void pic_irq_request(void *opaque, int irq, int lev= el) > hw/i386/pc.c-{ > hw/i386/pc.c- CPUState *cs =3D first_cpu; > hw/i386/pc.c- X86CPU *cpu =3D X86_CPU(cs); > [...] > hw/i386/pc.c: if (cpu->apic_state) { > [...] >=20 >=20 > Time to write a common pc_get_first_apic() helper, or provide a > PCMachineState::first_apic field? For this one, we could add a pc_get_apic_class() helper that returns NULL if there is no APIC on the first_cpu. It wouldn't be a change for this code and would be nicer for Radim's use case. Paolo