From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [V9 PATCH 7/8] pvh dom0: check for vioapic null ptr in vioapic_range Date: Mon, 21 Apr 2014 17:59:32 -0700 Message-ID: <20140421175932.3f93d0ee@mantra.us.oracle.com> References: <1397607172-32065-1-git-send-email-mukesh.rathor@oracle.com> <1397607172-32065-8-git-send-email-mukesh.rathor@oracle.com> <534EC6850200007800009B08@nat28.tlf.novell.com> <20140416184448.25c62fb1@mantra.us.oracle.com> <534F96DF0200007800009D57@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WcP3r-0005XV-BU for xen-devel@lists.xenproject.org; Tue, 22 Apr 2014 00:59:47 +0000 In-Reply-To: <534F96DF0200007800009D57@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: George.Dunlap@eu.citrix.com, tim@xen.org, eddie.dong@intel.com, keir.xen@gmail.com, jun.nakajima@intel.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Thu, 17 Apr 2014 07:54:55 +0100 "Jan Beulich" wrote: > >>> On 17.04.14 at 03:44, wrote: > > On Wed, 16 Apr 2014 17:05:57 +0100 > > "Jan Beulich" wrote: > > > >> >>> On 16.04.14 at 02:12, wrote: > >> > pvh doesn't use apic emulation, as a result vioapic_init is not > >> > called and vioapic ptr in struct hvm_domain is not initialized. > >> > One path that would access the ptr for pvh is : > >> > > >> > hvm_hap_nested_page_fault -> handle_mmio -> hvmemul_do_io -> > >> > hvm_mmio_intercept -> vioapic_range > >> > >> Given this I'm not sure the guard belongs here. The majority of the > >> handle_mmio() logic should never be used for Dom0. Perhaps you > >> should simply have a pvh_mmio_handlers[] paralleling > >> hvm_mmio_handlers[], but (presumably) only having HPET and MSI-X > >> entries for now? > > > > Well, there's already talk of adding vioapic support for PVH so it > > could take advantage of the new features coming up. So, it'll prob > > converge in near future with hvm_mmio_handlers . I'm ok either way. > > From a conceptual pov I still think the separation of emulation paths > should happen earlier and/or be more explicit, not the least because > iirc PVH guests are expected to not have a qemu associated. Correct. I've following for next version: diff --git a/xen/arch/x86/hvm/intercept.c b/xen/arch/x86/hvm/intercept.c index 7cc13b5..f89be28 100644 --- a/xen/arch/x86/hvm/intercept.c +++ b/xen/arch/x86/hvm/intercept.c @@ -42,6 +42,16 @@ hvm_mmio_handlers[HVM_MMIO_HANDLER_NR] = &iommu_mmio_handler }; +static const struct hvm_mmio_handler *const +pvh_mmio_handlers[HVM_MMIO_HANDLER_NR] = +{ + &hpet_mmio_handler, + NULL, + NULL, + &msixtbl_mmio_handler, + NULL, +}; + static int hvm_mmio_access(struct vcpu *v, ioreq_t *p, hvm_mmio_read_t read_handler, @@ -169,11 +179,13 @@ int hvm_mmio_intercept(ioreq_t *p) int i; for ( i = 0; i < HVM_MMIO_HANDLER_NR; i++ ) - if ( hvm_mmio_handlers[i]->check_handler(v, p->addr) ) - return hvm_mmio_access( - v, p, - hvm_mmio_handlers[i]->read_handler, - hvm_mmio_handlers[i]->write_handler); + { + const struct hvm_mmio_handler *mmio_handler = hvm_mmio_handlers[i]; + + if ( mmio_handler && mmio_handler->check_handler(v, p->addr) ) + return hvm_mmio_access(v, p, mmio_handler->read_handler, + mmio_handler->write_handler); + } return X86EMUL_UNHANDLEABLE; } > That aside - why is this coming up only now? The emulation path > getting reached shouldn't really depend on Dom0 vs Domu? The io emulation is handled by handle_pvh_io; there shouldn't be path for pvh domu leading to this function with all the restrictions and limitations it has at present. thanks mukesh