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:c0c::242; helo=mail-wr0-x242.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x242.google.com (mail-wr0-x242.google.com [IPv6:2a00:1450:400c:c0c::242]) (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 5EF992097412E for ; Tue, 26 Jun 2018 07:34:30 -0700 (PDT) Received: by mail-wr0-x242.google.com with SMTP id k16-v6so17523814wro.0 for ; Tue, 26 Jun 2018 07:34:30 -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=Wu5/N5KO9Y9XFpweXw/NlXeYb0LaFJanyaYsRG7Vg78=; b=IYnuypkG1AcUnHCqrMKnOwWofuCqI7xHjbVu2m5RWYPRsZ8SLN+R0fNzdNVAzchrPC SaBgX5OXBPVaHn/8L+LOEK4rGzGyIVSidHPwbAPvsEgZ/XE+fa8VkkLf9X7Sac85p3Kh nwPq9YH24iL6ylSkSXUbuIsijJ/7QChT/+GeQ= 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=Wu5/N5KO9Y9XFpweXw/NlXeYb0LaFJanyaYsRG7Vg78=; b=DdiySBmUaeN8fOQ1pljK7t9aBprVssVFlsJe/Jzzmmw1DWWx+vh+5JLg5P88pldc1q kKDUar0RoTwjAsyYplO8voJEKSrHIHurk+mIVXuGddszVNhVscdpmlHqF+smsdhWSCXu 0q42a5MnCIjLtCTaFXInBwp9BlQX/SbE/jgv4Wo+rDLWCQsDIwlMDwCeDc/QYnFE7EOq 0ukyEkcTpAf3jF403Gl+QPKwTdRLcRZjHLioH19TD3Rk/IUta1kwsshdOhM5gTufs40d iapEhw6xB99K9Nn9MYmpdgYbqyejpoGNokrJs5OxohovfGmCNOEQn3ybGEFs9QOH7p94 kEUw== X-Gm-Message-State: APt69E2aGskovlHeHIDACDWqnOknSE5ds7ntIrvYozPnxdStcHXEMnOI IhfIcIUuJaNBya+7TDncZ3MTTg== X-Google-Smtp-Source: AAOMgpecwdRt9l5zvqU1KaQiaIBqwTwCso7iPxTXRiAKewtY2fWkfx2Kc7gjIKVsRETClyUbnq91dw== X-Received: by 2002:adf:de08:: with SMTP id b8-v6mr1713200wrm.39.1530023668958; Tue, 26 Jun 2018 07:34:28 -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 c18-v6sm3061319wrq.17.2018.06.26.07.34.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 26 Jun 2018 07:34:27 -0700 (PDT) Date: Tue, 26 Jun 2018 15:34:26 +0100 From: Leif Lindholm To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" Message-ID: <20180626143426.b2bsv7hdjkjv7znd@bivouac.eciton.net> References: <20180626104424.3524-1-ard.biesheuvel@linaro.org> <20180626104424.3524-2-ard.biesheuvel@linaro.org> <20180626142409.gwn6daatv3vbnqxe@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation 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: Tue, 26 Jun 2018 14:34:31 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Jun 26, 2018 at 04:25:46PM +0200, Ard Biesheuvel wrote: > On 26 June 2018 at 16:24, Leif Lindholm wrote: > > On Tue, Jun 26, 2018 at 12:44:23PM +0200, Ard Biesheuvel wrote: > >> Add the basic support for enabling PCIe MMIO32 translation on the > >> SynQuacer, without actually enabling it just yet. It would allow us > >> to increase the bus range to 255 MB [from 127 MB] and the MMIO32 > >> range to 512 MB or more [from 128 MB], but it is more likely to > >> cause compatibility issues with code ported from the PC platform. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Ard Biesheuvel > >> --- > >> Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl | 8 ++++---- > >> Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h | 2 ++ > >> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c | 6 ++++-- > >> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 2 +- > >> 4 files changed, 11 insertions(+), 7 deletions(-) > >> > >> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl > >> index 51e9d0b22c3d..77d4763d1a85 100644 > >> --- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl > >> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl > >> @@ -86,14 +86,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR", > >> SYNQUACER_PCI_SEG0_BUSNUM_RANGE // RangeLength - # of Busses > >> ) > >> > >> - DWordMemory ( // 32-bit BAR Windows > >> + QWordMemory ( // 32-bit BAR Windows > >> ResourceProducer, PosDecode, > >> MinFixed, MaxFixed, > >> Cacheable, ReadWrite, > >> 0x00000000, // Granularity > >> SYNQUACER_PCI_SEG0_MMIO32_MIN, // Min Base Address > >> SYNQUACER_PCI_SEG0_MMIO32_MAX, // Max Base Address > >> - 0x00000000, // Translate > >> + SYNQUACER_PCI_SEG0_MMIO32_XLATE, // Translate > >> SYNQUACER_PCI_SEG0_MMIO32_SIZE // Length > >> ) > >> > >> @@ -224,14 +224,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR", > >> SYNQUACER_PCI_SEG1_BUSNUM_RANGE // RangeLength - # of Busses > >> ) > >> > >> - DWordMemory ( // 32-bit BAR Windows > >> + QWordMemory ( // 32-bit BAR Windows > >> ResourceProducer, PosDecode, > >> MinFixed, MaxFixed, > >> Cacheable, ReadWrite, > >> 0x00000000, // Granularity > >> SYNQUACER_PCI_SEG1_MMIO32_MIN, // Min Base Address > >> SYNQUACER_PCI_SEG1_MMIO32_MAX, // Max Base Address > >> - 0x00000000, // Translate > >> + SYNQUACER_PCI_SEG1_MMIO32_XLATE, // Translate > >> SYNQUACER_PCI_SEG1_MMIO32_SIZE // Length > >> ) > >> > >> diff --git a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h > >> index 950cece13e81..798f59db2a94 100644 > >> --- a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h > >> +++ b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h > >> @@ -34,6 +34,7 @@ > >> #define SYNQUACER_PCI_SEG0_MMIO32_MIN 0x68000000 > >> #define SYNQUACER_PCI_SEG0_MMIO32_MAX 0x6fffffff > >> #define SYNQUACER_PCI_SEG0_MMIO32_SIZE 0x08000000 > >> +#define SYNQUACER_PCI_SEG0_MMIO32_XLATE 0x0 > >> > >> #define SYNQUACER_PCI_SEG0_MMIO64_MIN 0x3e00000000 > >> #define SYNQUACER_PCI_SEG0_MMIO64_MAX 0x3effffffff > >> @@ -57,6 +58,7 @@ > >> #define SYNQUACER_PCI_SEG1_MMIO32_MIN 0x78000000 > >> #define SYNQUACER_PCI_SEG1_MMIO32_MAX 0x7fffffff > >> #define SYNQUACER_PCI_SEG1_MMIO32_SIZE 0x08000000 > >> +#define SYNQUACER_PCI_SEG1_MMIO32_XLATE 0x0 > >> > >> #define SYNQUACER_PCI_SEG1_MMIO64_MIN 0x3f00000000 > >> #define SYNQUACER_PCI_SEG1_MMIO64_MAX 0x3fffffffff > >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c > >> index 341939876bd3..7c096f0801dd 100644 > >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c > >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c > >> @@ -107,7 +107,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = { > >> SYNQUACER_PCI_SEG0_PORTIO_MAX, > >> MAX_UINT64 - SYNQUACER_PCI_SEG0_PORTIO_OFFSET + 1 }, // Io > >> { SYNQUACER_PCI_SEG0_MMIO32_MIN, > >> - SYNQUACER_PCI_SEG0_MMIO32_MAX }, // Mem > >> + SYNQUACER_PCI_SEG0_MMIO32_MAX, > >> + MAX_UINT64 - SYNQUACER_PCI_SEG0_MMIO32_XLATE + 1 }, // Mem > > > > So, this had me scratching my head for a second. > > I may get pickier about requring explicitly initializing the > > Translation field in future, but for this patch: > > Reviewed-by: Leif Lindholm > > > > That field did not exist yet when this code was merged. It was > introduced by Heyi's recent PCI patches. Ah, fair enough. Slightly unfortunate C just silently lets that through though... / Leif > >> { SYNQUACER_PCI_SEG0_MMIO64_MIN, > >> SYNQUACER_PCI_SEG0_MMIO64_MAX }, // MemAbove4G > >> { MAX_UINT64, 0x0 }, // PMem > >> @@ -127,7 +128,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = { > >> SYNQUACER_PCI_SEG1_PORTIO_MAX, > >> MAX_UINT64 - SYNQUACER_PCI_SEG1_PORTIO_OFFSET + 1 }, // Io > >> { SYNQUACER_PCI_SEG1_MMIO32_MIN, > >> - SYNQUACER_PCI_SEG1_MMIO32_MAX }, // Mem > >> + SYNQUACER_PCI_SEG1_MMIO32_MAX, > >> + MAX_UINT64 - SYNQUACER_PCI_SEG1_MMIO32_XLATE + 1 }, // Mem > >> { SYNQUACER_PCI_SEG1_MMIO64_MIN, > >> SYNQUACER_PCI_SEG1_MMIO64_MAX }, // MemAbove4G > >> { MAX_UINT64, 0x0 }, // PMem > >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c > >> index 227f9a725ce8..75a663e974e1 100644 > >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c > >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c > >> @@ -322,7 +322,7 @@ PciInitControllerPost ( > >> > >> // Region 0: MMIO32 range > >> ConfigureWindow (DbiBase, 0, > >> - RootBridge->Mem.Base, > >> + RootBridge->Mem.Base - RootBridge->Mem.Translation, > >> RootBridge->Mem.Base, > >> RootBridge->Mem.Limit - RootBridge->Mem.Base + 1, > >> IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM | > >> -- > >> 2.17.1 > >>