From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39093) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3IbC-0007PA-4b for qemu-devel@nongnu.org; Mon, 30 Nov 2015 02:10:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3Ib8-0002oZ-3e for qemu-devel@nongnu.org; Mon, 30 Nov 2015 02:10:10 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:26956) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3Ib7-0002kk-UW for qemu-devel@nongnu.org; Mon, 30 Nov 2015 02:10:06 -0500 Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout1.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NYM00BRY98OPR00@mailout1.w1.samsung.com> for qemu-devel@nongnu.org; Mon, 30 Nov 2015 07:10:00 +0000 (GMT) From: Pavel Fedin References: <48b59a60537a0dc5aac45b9015330eb641949ea5.1448359515.git.p.fedin@samsung.com> In-reply-to: Date: Mon, 30 Nov 2015 10:09:58 +0300 Message-id: <00f301d12b3e$203165d0$60943170$@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: quoted-printable Content-language: ru Subject: Re: [Qemu-devel] [RFC PATCH v3 1/5] hw/intc: Implement ITS base class List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Peter Maydell' Cc: 'Diana Craciun' , 'Shlomo Pongratz' , 'Shlomo Pongratz' , 'QEMU Developers' Hello! > > + /* Our two regions are always adjacent, therefore we now = combine them > > + * into a single one in order to make our users' life easier. > > + */ > > + memory_region_init(&s->iomem_main, OBJECT(s), "gicv3_its", = ITS_SIZE); > > + memory_region_add_subregion(&s->iomem_main, 0, = &s->iomem_its_cntrl); > > + memory_region_add_subregion(&s->iomem_main, ITS_CONTROL_SIZE, > > + &s->iomem_its); > > + sysbus_init_mmio(sbd, &s->iomem_main); >=20 > So we have two memory subregions: > * the control register page, whose ops are passed in by the subclass > * the translation register page, whose only register is implemented > here by looking up the subclass and calling its send_msi method >=20 > Does that complexity gain us anything later? There doesn't really > seem to be anything much actually in common here, which would > suggest just having the subclasses create their own mmio region(s) > (which could then just directly implement the right behaviour for > GITS_TRANSLATER). I started from this, but decided to move region creation into base = class, because: 1. We always have this region, both for KVM and for SW implementation, = and it operates in the same way. 2. We always need to do address validation, as well as have = gicv3_its_trans_read() stub. 3. Also, in the next revision i'll implement 16-bit handling here. So, i decided not to duplicate these things. > > + > > +const char *its_class_name(void) > > +{ > > + if (kvm_irqchip_in_kernel()) { > > +#ifdef TARGET_AARCH64 > > + /* KVM implementation requires this capability */ > > + if (kvm_direct_msi_enabled()) { > > + return "arm-its-kvm"; > > + } > > +#endif >=20 > Why is this in an #ifdef? In theory we could support > the GICv3 in a 32-bit system. Only for code consistency, because "arm-its-kvm" class is compiled only = for TARGET_AARCH64, because only there we have the relevant kernel API = definitions. So, i did not want to refer to nonexistent class. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia