From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::241; helo=mail-io0-x241.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x241.google.com (mail-io0-x241.google.com [IPv6:2607:f8b0:4001:c06::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 7CE7D209603EF for ; Thu, 31 May 2018 03:49:08 -0700 (PDT) Received: by mail-io0-x241.google.com with SMTP id e15-v6so16875047iog.1 for ; Thu, 31 May 2018 03:49:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=hdENeGAebk6Zhkj8kGvsGfExPb+HVAbQXpyvV+3IVYA=; b=UW7vGUmZ62UPILcQeWxZ9qpf/Hn4o2O+3f3HDyMrjooqQAQLu3/tk/m+fQ+zm3VNBL v8MYIYcY6WKiUqBZcMu12kGbOtiUqwpxObDYIaTDhCs8Eji9akIeMdKcMqXi9QMZJlRe 0vBBoYcoFJK81x4sCSEtVlpo6eSy/nUTRnE1M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=hdENeGAebk6Zhkj8kGvsGfExPb+HVAbQXpyvV+3IVYA=; b=JQjJyQOIVE+P807NyCrM8I4oP6Z16FY6Tef9XBW1hiQj4rzD/rGxnaRk7O0j+LPXNo uEWHVRKFCrlSz+qEZPhL3cwOYpR4VY+BJaKOVbTdPKqOZJk/vvMxJL7G47jkflE2g+QL F7m657skHxtgUeGmCLWH70LAVDDq8/tWnfNZOLi+cTRr0wdweo2wqqE3eV7u1G5DL0+c LMeLOMNBj4lExKvXA3mzKLORDb6QriECFGUQzZlB/92AxBrfoTOqfbfT4SBYdZ9GZdX6 sUv31yaVoj4JKSzJyltmS1XdTWXXzKLxMCl83ml8ciCuHbeGlLn0TXi94OFlBZKhjpyu Xunw== X-Gm-Message-State: ALKqPwdP6eYcsBn7stfHPVYF14kaIsVvPtsgiGME9brrBKE0KQL6PB2U SVh7KLrAMk/w1MN88ocpOfaKxXhIBjLzgxNRcQ7PSZolevI= X-Google-Smtp-Source: ADUXVKKJ7I+27B7ephp9qsSA9YDUbK81+cllmwXhpTz8sEnofTwNSlz8VrV6lIfmWj9ntRraQN/GMqAmoBF93nD/6k4= X-Received: by 2002:a6b:268b:: with SMTP id m133-v6mr6141256iom.107.1527763747591; Thu, 31 May 2018 03:49:07 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bb86:0:0:0:0:0 with HTTP; Thu, 31 May 2018 03:49:07 -0700 (PDT) In-Reply-To: <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> <20180531094623.lpisubt3gorz2kk2@bivouac.eciton.net> From: Ard Biesheuvel Date: Thu, 31 May 2018 12:49:07 +0200 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Masahisa Kojima 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 10:49:08 -0000 Content-Type: text/plain; charset="UTF-8" On 31 May 2018 at 11:46, Leif Lindholm wrote: > 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 > Thanks Pushed as c9be7b11ea10 (with my S-o-b added)