From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::241; helo=mail-wm0-x241.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2E9B720977871 for ; Thu, 31 May 2018 02:46:27 -0700 (PDT) Received: by mail-wm0-x241.google.com with SMTP id z6-v6so1055391wma.0 for ; Thu, 31 May 2018 02:46:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=4wwV1sut0gnPM32NdKSZmpmZxnCBEbharsJrDCwogYM=; b=i0LH5NkBXlHkdvBcLOb3x8nSVZLg6LDbx164jAW0O8tvChX3P+E0cRGpRAnMvRq/4N BQbszaoTXBOI84WcBl3Eiuj1o3hPRiFsO1ziEkGZicV/2NDf4DwdIMlGV06xXbCuF7Tb pZfWQm/7mOZQS7rAtDZU1nM5DKQghRZCBvt44= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=4wwV1sut0gnPM32NdKSZmpmZxnCBEbharsJrDCwogYM=; b=PhULiKjGPpc3lcyF6RvSHlupP/1eDz6Z7p5PWvBWR29L9N7//2oFltzVBK2aGBp9vW 3kNVPxMPV3PiyeOWLt4ZYMLj/y3Tk1wXfKWHShAiLUb5jFmuRurZyO88hGEg1EnGimqV 2U45o9zsnD9rl+ZJO+Dpt8O2Mw5dpzwf2ztDLv3JLIgfp8xGb4ktgGgZ6S2FOTZosoKK umoT3l7QN2e5QeB6PhOhSPulfNO3KveFiABkPEhxeY4SOr4XuVxah6hERvyEf/8ZjnRe IGUXgyfgCiCa1uOEk1l2b/hLywTKfxC0Iwp7HksAMXwR0lYGNvEpNbJwQF553FVVw5zW ageQ== X-Gm-Message-State: ALKqPwcvYPfM+dvMFX+zUURwgcVjhHfrJ3YzO+g4v8xG+JlExSNeHc6I K4WRjmbUw74UJb+uztTckwq/SQ== X-Google-Smtp-Source: ADUXVKID354rMkjL38qRxfXacjxPxvUAXO75o1WcgwCWqwOkoOB+1z7eEl2Gb9/DWS1ZLWyzLkY6gA== X-Received: by 2002:a50:9235:: with SMTP id i50-v6mr2259710eda.27.1527759986395; Thu, 31 May 2018 02:46:26 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id c11-v6sm10401934eda.64.2018.05.31.02.46.24 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 31 May 2018 02:46:25 -0700 (PDT) Date: Thu, 31 May 2018 10:46:23 +0100 From: Leif Lindholm To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , Masahisa Kojima Message-ID: <20180531094623.lpisubt3gorz2kk2@bivouac.eciton.net> References: <20180530181929.5066-1-ard.biesheuvel@linaro.org> <20180530181929.5066-2-ard.biesheuvel@linaro.org> <20180531091120.u3oqbzskpr6rnhen@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 May 2018 09:46:28 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, May 31, 2018 at 11:17:47AM +0200, Ard Biesheuvel wrote: > On 31 May 2018 at 11:11, Leif Lindholm wrote: > > On Wed, May 30, 2018 at 08:19:27PM +0200, Ard Biesheuvel wrote: > >> From: Masahisa KOJIMA > >> > >> The current revision of SC2A11 contains PCIe bus issue. > >> In MRd transaction, 1st/Last DW BE fields are not correctly set > >> by hardware. > >> > >> As a workaround, set TH bit and specify MSG_CODE in iATU. > >> With this setup, the value specified as MSG_CODE is set to the > >> 1st/Last DW BE fields and PCIe controller can emit the correct > >> MRd TLP header. > >> Same workaround was already included for MMIO32 region, > >> MMIO64 region also requires this workaround. > >> Some deivices, such as Samsong SSD 970 EVO, do not work > >> without this modification. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Masahisa KOJIMA > > > > Please add own S-o-b. > > > >> --- > >> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c > >> index e4679543cc66..227f9a725ce8 100644 > >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c > >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c > >> @@ -359,8 +359,9 @@ PciInitControllerPost ( > >> RootBridge->MemAbove4G.Base, > >> RootBridge->MemAbove4G.Base, > >> RootBridge->MemAbove4G.Limit - RootBridge->MemAbove4G.Base + 1, > >> - IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM, > >> - 0); > >> + IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM | > >> + IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TH, > >> + IATU_REGION_CTRL_2_OFF_OUTBOUND_0_MSG_CODE_32BIT); > > > > Hmm ... > > This fix clearly needs to go in. But since this is working around a > > bug in first-revision silicon, should we not have something > > conditional here? > > In theory, yes. In practice, we have no idea yet whether a fixed > revision will ever materialize (the limited respin for the next > revision does not address this issue afaik), nor do we have any idea > how to distinguish them at runtime. This is clearly a problem, but even if we can't distinguish them, that could be made a menu setting (since getting it wrong won't prevent you from going into the menu and changing it back). > Also, it is unlikely that we will > need to run older firmware builds on these new chips. So for the time > being, I think it is reasonable to apply this unconditionally (like we > do for the MMIO32 region already) My opinion stands. But if we already haven't conditionalised the MMIO32 workaround, then I guess we don't need to tear up the world at this point. But I'll just file this forward request that workarounds for hardware bugs are always conditionalised in future, so that 1) it's crystal clear what bits are workarounds 2) they can more easily be disabled when functional hardware appears. Anyway Reviewed-by: Leif Lindholm / Leif