From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::62e; helo=mail-pl1-x62e.google.com; envelope-from=bcr@google.com; receiver=edk2-devel@lists.01.org Received: from mail-pl1-x62e.google.com (mail-pl1-x62e.google.com [IPv6:2607:f8b0:4864:20::62e]) (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 E52E921B02822 for ; Wed, 21 Nov 2018 19:05:48 -0800 (PST) Received: by mail-pl1-x62e.google.com with SMTP id b22-v6so8267860pls.7 for ; Wed, 21 Nov 2018 19:05:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ZFmcdBKynCv2gdfdsSA/8k8PPwnostfVWJcX0fHJrr0=; b=Uke9Imum4bD7gYCftYyAj423JK2cHxSqCdTRhnjmbbgtNv9w3GaqXuUX/tq2HYRHny I0C+DoAUTTdpJCV1+ZI8OMllfgjHy9u5XlOzem4WKHC5F1zzWRJdWE/2EIgZBa7joqnM UrUmBCt9QQiLnA3zfqmqmL/xlzNsHVKggtwSgWVRwbSIKee3FzrWHFM0wtOo8Qg1gBhL XQHS/aO/RgXUNJD4d75k5lM7JwsKtzplvL0zXj+b7Whn4qe3m33EQ3u+fx+wIfSAjKXI R/zt1yti6EkXbKDIM51IEAEtoZ901IXAFFOysxL0Hu1nropwUEiixcn1LaQPow1I4723 Hrsg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ZFmcdBKynCv2gdfdsSA/8k8PPwnostfVWJcX0fHJrr0=; b=lhYEs7CWCZIXjZJlMr51VNEiJCST9lec3Nd6MmN6ElE8Rwk0GBm+mzSdPFCJmM9LAG +dzg1OAPxnTo2JBeka+KNsAkEZVzAm7cMM3hfWv8GYrIOuga+Ymz6rT10caKVi8vwmwH csees+CYsedFhZWsk1wSvHFXyH7taoZ7OxXin0xFXC7jpXSVCUCayRchwYwX+twnBZLq jBsaz0hok4Bm7UfDeJ2vPJuJBoKTUZneubhYDRax4fuwO50+xjl4VTPOdACk9KLgDOl5 Clmx34aw0AwEU6ZcloorRmVVxdo7NNZhP4QIhFRzy4CXxQpj06H1xhK2u1IaZNu0YD/F AtZA== X-Gm-Message-State: AGRZ1gIFtZx+3TN+8A3s0SBRsYZm69zOTDYKN8ZJsgRPKCvkHcx0Iu0Q loRodQNsnhcaih6D79/+O4C4xCeOzI6dEaPDdbIJsw== X-Google-Smtp-Source: AJdET5edxEkjXmFHsvZC190M3pBAO722X9Bz/qiiu5tjyH2IHJha1d0MGQ/jRKk95YhyWAzluDcGf6frurYaiRg1r3s= X-Received: by 2002:a62:546:: with SMTP id 67mr9336508pff.99.1542855948193; Wed, 21 Nov 2018 19:05:48 -0800 (PST) MIME-Version: 1.0 References: <8147e54ded6e405abdafbed45bd52199@SCL-EXCHMB-13.phoenix.com> In-Reply-To: From: Bryan Rosario Date: Wed, 21 Nov 2018 19:05:36 -0800 Message-ID: To: Ken_Taylor@phoenix.com Cc: Alain Gefflaut , edk2-devel@lists.01.org X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: Hardcoded IDE Controller B/D/F (0/1/1) in BdsPlatform.c? X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Nov 2018 03:05:49 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable This is for a virtual x86 platform using OVMF. I think I have enough info at this point to get things set up. Thanks for the help. Bryan On Wed, Nov 21, 2018 at 6:06 PM Ken Taylor wrote: > Hi Bryan, > > > > If you were talking about PC architecture, I would suggest that there > should be a hierarchy of configuration that needs to occur. Specifically= , > there should be silicon / board / platform specific configuration that is > architecturally independent of the SATA driver. > > > > Normally, there would be early initialization to write to silicon specifi= c > registers. Later, the PCI bus driver should expose PCI device interfaces= . > That driver would normally be responsible for writing to the PCI > configuration header for the device, as well as for configuring any PCI > bridges between the host bus and the device. > > > > The SATA driver would then connect to the PCI device interface exposed by > the PCI bus driver. > > > > However, I think you=E2=80=99re using an ARM based system. The architect= ure is > different from what I am used to. Is what you have is a SATA driver > talking directly to the PCI device interface? This skips several levels = of > the architecture that would be responsible for maintaining platform > independence. > > > > If it were me, I would only put generic SATA code in the SATA driver. > There wouldn=E2=80=99t be anything silicon or platform specific at all. > > > > If we were talking about the old IDE interfaces (because CF maybe), there > used to be stuff for configuring 80 conductor vs 40 conductor cables that > was handled by legacy BIOSes, and configuring high speed DMA and PIO > transfer modes was silicon specific. In the legacy architectures I worke= d > on, there was usually an interface for silicon specific configuration tha= t > was architecturally independent of the IDE driver itself. > > > > If you are facing the same sort of thing, then I would define a protocol > for features that may require silicon specific register writes, and make = my > SATA code call that protocol. This allows you to keep all your > silicon/board/platform specific code in one location (perhaps one per > design), and you don=E2=80=99t have to touch as many files every time you= r hardware > design changes. Ideally, you should have all the things that need to > change boiled down to one particular piece of code, so you can focus on > what you need to do to make new hardware work when you get new hardware a= nd > don=E2=80=99t have to discover all sorts of files that need editing piece= meal as > you discover non-functional hardware features. > > > > Regards, > > -Ken. > > > > *From:* Bryan Rosario [mailto:bcr@google.com] > *Sent:* Wednesday, November 21, 2018 5:29 PM > *To:* Ken Taylor > *Cc:* Alain Gefflaut; edk2-devel@lists.01.org > *Subject:* Re: [edk2] Hardcoded IDE Controller B/D/F (0/1/1) in > BdsPlatform.c? > > > > Hi Ken, > > > > That's what I had guessed -- thanks for confirming that it's platform > specific. > > > > In my use case, I actually want the code to work on multiple hardware > configurations, so I can't just replace the hardcoded B/D/F with the one > used by a particular platform. Does the change to SataController.c that I > mentioned in the original email make sense? I've tested it and confirmed > that it works on my platform, but I want to get a sanity check that that = is > an appropriate place to put this code. > > > > Thanks, > > Bryan > > > > On Wed, Nov 21, 2018 at 4:48 PM Ken Taylor wrote= : > > Hi Bryan, > > That appears to be silicon or platform specific code. It's intended for > one specific hardware configuration. Unless you have an identical hardwa= re > configuration, those particular register writes are actually nonsensical. > > I think what you need to do is consult the silicon specific documentation > for your particular hardware configuration, and determine what silicon > specific registers need to be configured to what specific values to suppo= rt > your specific hardware design. > > Regards, > -Ken. > > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Bryan Rosario via edk2-devel > Sent: Wednesday, November 21, 2018 4:07 PM > To: edk2-devel@lists.01.org > Cc: Alain Gefflaut > Subject: [edk2] Hardcoded IDE Controller B/D/F (0/1/1) in BdsPlatform.c? > > Hi all, > > I noticed that there is apparently a hardcoded B/D/F for an IDE Controlle= r > in BdsPlatform.c, when setting bit 15 of a particular register: > > https://github.com/tianocore/edk2/blob/14923c1a6bf9940b48feeaf47cb5d6c662= b6528c/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c#L1612-L1617 > . > > Why is this hardcoded? Is it just old code that has hung around? I notice= d > that this code is from this commit, and the surrounding code at the time > had lots of hardcoded B/D/Fs: > > https://github.com/tianocore/edk2/commit/40f2c454343be84ab3bacf9955cc8d78= 42c70b5c > . > > The context of this question is that I'm trying to make this work in a > configuration with an IDE Controller located at a different B/D/F, and so > the hardcoded value of 0/1/1 doesn't work for me. > > If I want to change this so that it's not hardcoded, what is a good > approach? I've added some code locally to the IdeInitSetTiming function i= n > SataController.c to set the bit using the PciIo structure which is opened > specifically on the controller in question -- does that sound like the > right approach? > Link to the function I'm referring to: > > https://github.com/tianocore/edk2/blob/f6b0258d25a63ae3d3bc6430abe30fb625= abc52a/OvmfPkg/SataControllerDxe/SataController.c#L1091-L1099 > > Thanks, > Bryan > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > >