From patchwork Wed Aug 26 20:36:27 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alistair Francis X-Patchwork-Id: 511049 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id A074A14075D for ; Thu, 27 Aug 2015 06:37:19 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=T3jOMxBB; dkim-atps=neutral Received: from localhost ([::1]:41353 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUhRd-0006qZ-Od for incoming@patchwork.ozlabs.org; Wed, 26 Aug 2015 16:37:17 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39038) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUhRL-0006Z5-Uq for qemu-devel@nongnu.org; Wed, 26 Aug 2015 16:37:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUhRK-00054x-HC for qemu-devel@nongnu.org; Wed, 26 Aug 2015 16:36:59 -0400 Received: from mail-oi0-x22e.google.com ([2607:f8b0:4003:c06::22e]:35006) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUhRK-00054o-AU for qemu-devel@nongnu.org; Wed, 26 Aug 2015 16:36:58 -0400 Received: by oifo84 with SMTP id o84so3024120oif.2 for ; Wed, 26 Aug 2015 13:36:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc:content-type:content-transfer-encoding; bh=mu0zQ0bfqAivybjL4uOtgpX6c7zYBZYKl3Tp8YBwzy8=; b=T3jOMxBBAMPHyAcWigZxo3qSCYz5GjmJFht2yJPhf/786vyiIs54q9wJX7d92AVxRN bkHXQibARbpRSNWZ14o5MSCEhjL+tp8r+v4sibQSRBRduloZyhqSNRTqWkyA5J/cR695 Jep6/c+W6FBX/wprwdSsTaesaMe9UeQsC9X4oh/705lKuUS91tfG+veFqptdGXMabLiZ LIEveTvNNKFPj6n/RqRkvsG8WHZLUQHb2/AH8oEp9qgXu48lFIjzudACzp9o14kSKDq9 s9e6YmqhDrPUs5et9auteX09QTEWTvpmhnM6rQRUwTLf25hpEf8BkvZDk/KMshZEvlj4 rwig== X-Received: by 10.202.172.87 with SMTP id v84mr280078oie.51.1440621417512; Wed, 26 Aug 2015 13:36:57 -0700 (PDT) MIME-Version: 1.0 Received: by 10.182.56.135 with HTTP; Wed, 26 Aug 2015 13:36:27 -0700 (PDT) In-Reply-To: References: <76f0a328e53daa4a2303fe17d3bbe157d7b40d03.1437699224.git.alistair.francis@xilinx.com> <55D26157.8050502@suse.de> From: Alistair Francis Date: Wed, 26 Aug 2015 13:36:27 -0700 X-Google-Sender-Auth: gsCtdTfxfHg6jnMXG-vgpiwoQqA Message-ID: To: Peter Crosthwaite X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:4003:c06::22e Cc: Edgar Iglesias , Peter Maydell , mst@redhat.com, "qemu-devel@nongnu.org Developers" , Alistair Francis , Sai Pavan Boddu , =?UTF-8?Q?Andreas_F=C3=A4rber?= Subject: Re: [Qemu-devel] [PATCH v1 2/3] object.c: object_class_dynamic_cast return NULL if the class has no type X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Tue, Aug 25, 2015 at 12:43 AM, Peter Crosthwaite wrote: > On Mon, Aug 24, 2015 at 4:36 PM, Alistair Francis > wrote: >> On Mon, Aug 17, 2015 at 4:37 PM, Peter Crosthwaite >> wrote: >>> On Mon, Aug 17, 2015 at 3:33 PM, Andreas Färber wrote: >>>> Am 18.08.2015 um 00:24 schrieb Alistair Francis: >>>>> On Sat, Aug 15, 2015 at 2:22 PM, Peter Crosthwaite >>>>> wrote: >>>>>> On Mon, Jul 27, 2015 at 11:37 AM, Alistair Francis >>>>>> wrote: >>>>>>> If the ObjectClass has no type return NULL instead of trying to compare >>>>>>> the type name. >>>>>>> >>>>>> >>>>>> What was the issue? >>>>> >>>>> There is a seg fault in object_class_dynamic_cast() because there is >>>>> no type in the ObjectClass struct. >>>> >>>> That should never happen, ever since TYPE_OBJECT is no longer NULL. >>>> >>>>> It happens when it is trying to cast the "pci-device", which is called >>>>> from the ahci_irq_lower() function. The function is testing if the >>>>> device is a pci device, so it should return NULL if it isn't valid. >>> >>> Yes so I vaguely remember this now. It is about MSI interrupts which >>> have nothing to do with sysbus implementation. My solution was to rip >>> that PCI specific stuff out of AHCI completely in my branch. Should >>> sysbus and PCI AHCI classes install their own separate logic for this >>> part via a virtualised hook? >>> >>> On the topic though, I notice many PCI devices have this MSI specific >>> logic in them. Is it possible for devs to just treat interrupts as >>> pins and the PCI layers do the MSI vs non-MSI logic switch in core >>> layers? >>> >>> If Andreas' idea don't work this is still a core QOM bug though. I >>> think object_dynamic_cast should not have this segfault when passed a >>> non implementing object. >>> >>> Regards, >>> Peter >>> >>>> >>>> It rather sounds as if some build-time dependency is wrong, which we >>>> used to run into for the Container type before Paolo macrofied this. >>>> >>>> Please try again with a clean build - if it still occurs, we'll need a >>>> reproducible test case to investigate what is going on rather than >>>> papering over a latent bug. >> >> Hey, >> >> Sorry abut the delay, but I didn't get a chance to look at this last >> week. I tried with a clean setup and still see the seg fault. >> >> I will try to look into it more this week, but if anyone is interested >> here are the steps to reproduce: >> >> On the latest mainline QEMU, with my 2nd and 3rd patches applied >> $ ./configure --target-list="aarch64-softmmu,microblazeel-softmmu" >> --disable-pie --disable-sdl --disable-werror # This is what is >> required at work >> $ ./aarch64-softmmu/qemu-system-aarch64 -M xlnx-ep108 -display none >> -kernel ./u-boot.elf -m 8000000 -nographic -serial mon:stdio # Boot >> u-boot on QEMU >> >> The image I'm using is available at: http://1drv.ms/1NxDXLo >> > > So it's not a core bug. That container_of in ahci_lower_irq is > incorrectly assuming that the passed AHCIState * is always for a PCI, > which it is not in the sysbus case. So it's incorrectly getting the > offset of QOM the object and the QOM cast is treating some invalid > offset into the (or past) object as a QOM object base address. > > The simplest solution is a back pointer in AHCIState to the > encapsulating device (would be a DeviceState *). The container_of is > replaced with a nav of this pointer and then the conditional PCI cast > can work. This seems to fix the problem. It seems hacky though, I can't find a better way to check the validity of the PCIDevice. Any ideas? Thanks, Alistair > > Regards, > Peter > >> Thanks, >> >> Alistair >> >>>> >>>> Thanks, >>>> Andreas >>>> >>>> -- >>>> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >>>> GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg) >>> > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 02d85fa..77e58a9 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -137,8 +137,11 @@ static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev) static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev) { AHCIPCIState *d = container_of(s, AHCIPCIState, ahci); - PCIDevice *pci_dev = - (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE); + PCIDevice *pci_dev = NULL; + + if (s->parent_obj) { + pci_dev = PCI_DEVICE(d); + } DPRINTF(0, "lower irq\n"); diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index c055d6b..ac7d2de 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -287,6 +287,8 @@ struct AHCIDevice { }; typedef struct AHCIState { + DeviceState *parent_obj; + AHCIDevice *dev; AHCIControlRegs control_regs; MemoryRegion mem;