From patchwork Tue Mar 31 22:34:00 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Cave-Ayland X-Patchwork-Id: 457043 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 00D0214008F for ; Wed, 1 Apr 2015 09:34:41 +1100 (AEDT) Received: from localhost ([::1]:40852 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yd4k3-00055s-4k for incoming@patchwork.ozlabs.org; Tue, 31 Mar 2015 18:34:39 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45288) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yd4jh-0004bb-Tw for qemu-devel@nongnu.org; Tue, 31 Mar 2015 18:34:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yd4jg-0001Qc-WC for qemu-devel@nongnu.org; Tue, 31 Mar 2015 18:34:17 -0400 Received: from s16892447.onlinehome-server.info ([82.165.15.123]:54340) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yd4jc-0001Q5-BW; Tue, 31 Mar 2015 18:34:12 -0400 Received: from 4e567451.skybroadband.com ([78.86.116.81] helo=[192.168.1.81]) by s16892447.onlinehome-server.info with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1Yd4jX-00033o-7J; Tue, 31 Mar 2015 23:34:08 +0100 Message-ID: <551B20D8.3010806@ilande.co.uk> Date: Tue, 31 Mar 2015 23:34:00 +0100 From: Mark Cave-Ayland User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.5.0 MIME-Version: 1.0 To: Paolo Bonzini , Peter Crosthwaite References: <5516FEDE.3030809@ilande.co.uk> <55191C97.8070308@redhat.com> <5519235C.6090004@ilande.co.uk> <5519256A.1010202@redhat.com> <551937D6.6060904@redhat.com> In-Reply-To: <551937D6.6060904@redhat.com> X-SA-Exim-Connect-IP: 78.86.116.81 X-SA-Exim-Mail-From: mark.cave-ayland@ilande.co.uk X-SA-Exim-Version: 4.2.1 (built Sun, 08 Jan 2012 02:45:44 +0000) X-SA-Exim-Scanned: Yes (on s16892447.onlinehome-server.info) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 82.165.15.123 Cc: Peter Maydell , qemu-ppc Mailing List , "qemu-devel@nongnu.org Developers" Subject: Re: [Qemu-devel] ppc vga output breakage since commit c3c1bb99 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 30/03/15 12:47, Paolo Bonzini wrote: > On 30/03/2015 13:45, Peter Crosthwaite wrote: >> Can the address_space_translate_address() length clamp be made >> conditional on non-MMIO access as the RC fix? I submitted >> c3c1bb99d1c11978d9ce94d1bdcf0705378c1459 as I think its the right >> thing to do regardless of memory type, but in reality it only fixes a >> bug I encountered with RAM memory regions. The original code ignores >> address_space_translate_internal() return-by-pointer length value >> absolutely and the new code uses it absolutely. Should we just if the >> whole thing, old vs new behaviour on MMIO vs non-MMIO? >> >> Happy to submit that fixup if that's the accepted plan. > > I submitted what I think is the right fix (sorry Mark for misspelling > your email address). I think you're patch is correct, so I'd rather not > introduce hacks for the release; we can either revert it, or fix ioport.c. Ah wait a second - the same patch c3c1bb99 also breaks reset on SPARC32: $ ./qemu-system-sparc -prom-env 'auto-boot?=false' -nographic Configuration device id QEMU version 1 machine id 32 Probing SBus slot 0 offset 0 Probing SBus slot 1 offset 0 Probing SBus slot 2 offset 0 Probing SBus slot 3 offset 0 Probing SBus slot 4 offset 0 Probing SBus slot 5 offset 0 Invalid FCode start byte CPUs: 1 x FMI,MB86904 UUID: 00000000-0000-0000-0000-000000000000 Welcome to OpenBIOS v1.1 built on Mar 12 2015 08:08 Type 'help' for detailed information 0 > reset-all Unhandled Exception 0x00000029 PC = 0xffd10c08 NPC = 0xffd10c0c Stopping execution The reset is controlled by writing a 1 to the hardware reset register at physical address 0x71f00000 (system-control according to "info mtree") which is handled in hw/misc/slavio_misc.c. Given that SYSCTRL_SIZE isn't used at all (and the access size for slavio_led_mem_ops also looks wrong), the following seems to be the correct fix for the reset and LED access: /* AUX 1 (Misc System Functions) */ Then again it is getting quite late in the release cycle... ATB, Mark. --- a/hw/misc/slavio_misc.c +++ b/hw/misc/slavio_misc.c @@ -68,6 +68,7 @@ typedef struct APCState { } APCState; #define MISC_SIZE 1 +#define LED_SIZE 2 #define SYSCTRL_SIZE 4 #define AUX1_TC 0x02 @@ -452,13 +453,13 @@ static int slavio_misc_init1(SysBusDevice *sbd) /* 16 bit registers */ /* ss600mp diag LEDs */ memory_region_init_io(&s->led_iomem, OBJECT(s), &slavio_led_mem_ops, s, - "leds", MISC_SIZE); + "leds", LED_SIZE); sysbus_init_mmio(sbd, &s->led_iomem); /* 32 bit registers */ /* System control */ memory_region_init_io(&s->sysctrl_iomem, OBJECT(s), &slavio_sysctrl_mem_ops, s, - "system-control", MISC_SIZE); + "system-control", SYSCTRL_SIZE); sysbus_init_mmio(sbd, &s->sysctrl_iomem);