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:c0b::236; helo=mail-it0-x236.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x236.google.com (mail-it0-x236.google.com [IPv6:2607:f8b0:4001:c0b::236]) (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 E946B209574F7 for ; Tue, 27 Feb 2018 02:08:48 -0800 (PST) Received: by mail-it0-x236.google.com with SMTP id o9so14472753itc.1 for ; Tue, 27 Feb 2018 02:14:54 -0800 (PST) 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=7t25RmWrJU4cEEzrx+2SAZSfaE3UDshrfwQ9xP/Ju6A=; b=Vp26m6qA3Xj8MKCFJwW+AKfayZ7MpwMP0LdQcnkd4lzDCRiltaEUXmc/C8YnnD33J2 VOMuY7bGIyPKK9hKyvPm6vH/EX5W9UdJVgqAlAbdjlb5QUb7lEeX1NlUXAs0oelUOWSl 2gqWqu+qG7RKz/uGx/st8WvcCeSMn8xQ4r47c= 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=7t25RmWrJU4cEEzrx+2SAZSfaE3UDshrfwQ9xP/Ju6A=; b=IOC12qgchclb7JEQTrd4K9tCBhB9zeOLfzRpl7JPL1NZgDCkQq/GGeVxow5F13cssr F48USQxK+fdUTMkwnXRFWfYvkFYST47fgFPknTOV1UQddi8l/nyl5wDKS/XRXhgmaxek rvWb8p2r28Nt6TdnXLEWci1JZ4qf8E53Er5lZ6Jla+Z0X4Kqnj+e2RZz/yorqbxceL46 qJazL4ZrG/qs3H70lzbWTAay1lRToqTSS7kPlA6eDFHGkoQUgaJJePjQMkBwe7Y6hdrY jzDwjeaujXCkInHAAmjDnqWz+Uw4MDb1Uswj0dyZc/YXYQz8Kgf8sIlNKBIswc4uAPwp sKwQ== X-Gm-Message-State: APf1xPA4/7X8PNghhPE0n3HP12yqMZheYcLn6vpjt5iVPD+FfTnD9iiK lXHPoqvz4ib1d4gcG5+5B95tfPyCzNJaJUKdzAYhWA== X-Google-Smtp-Source: AH8x224FHo1+QNTNV9ZEvM21CyZHveQWj100ZUhodRtJ4mJ9L7EbwWz8HjAQX0XGo6383mrn/XsujkpvjSVZ6WHRXzI= X-Received: by 10.36.217.22 with SMTP id p22mr15976117itg.106.1519726493907; Tue, 27 Feb 2018 02:14:53 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.138.209 with HTTP; Tue, 27 Feb 2018 02:14:53 -0800 (PST) In-Reply-To: <19deadb3-4d26-914c-c9f8-c6c6716746fa@Intel.com> References: <1519697389-3525-1-git-send-email-heyi.guo@linaro.org> <1519697389-3525-2-git-send-email-heyi.guo@linaro.org> <563e76b9-ebbc-be09-d306-8dcb86f12112@Intel.com> <20180227093334.GA3918@SZX1000114654> <19deadb3-4d26-914c-c9f8-c6c6716746fa@Intel.com> From: Ard Biesheuvel Date: Tue, 27 Feb 2018 10:14:53 +0000 Message-ID: To: "Ni, Ruiyu" Cc: Guo Heyi , Eric Dong , "edk2-devel@lists.01.org" , Michael D Kinney , Laszlo Ersek , Star Zeng Subject: Re: [RFC v4 1/3] MdeModulePkg/PciHostBridgeDxe: Add support for address translation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Feb 2018 10:08:49 -0000 Content-Type: text/plain; charset="UTF-8" On 27 February 2018 at 09:59, Ni, Ruiyu wrote: > On 2/27/2018 5:33 PM, Guo Heyi wrote: >> >> Hi Ray, >> >> Thanks for your comments. It seems comments 10 and 12 are the major issue; >> let's >> discuss that first. >> >> 1. In current implementation, namely PciBusDxe and PciHostBridgeDxe both >> in >> MdeModulePkg, Address parameter passed to RootBridgeIoMemRead comes from >> PciIoMemRead(). >> 2. In PciIoMemRead(), Offset is only an offset within selected BAR; then >> PciIoDevice->PciBar[BarIndex].BaseAddress is added to Offset in >> PciIoVerifyBarAccess(). > > Agree. I thought PciIoMemRead() gets the BAR base address from > GetBarAttributes(). > >> 3. So whether the "Address" passed to RootBridgeIoMemRead() is host >> address or >> device address depends on the property of >> PciIoDevice->PciBar[BarIndex].BaseAddress. > > RootBridgeIoMemRead() can choose to accept HOST address or Device address. > Either can be implemented. > I am fine to your proposal to use Device Address. > >> 4. In PciBusDxe, we can see PciIoDevice->PciBar[BarIndex].BaseAddress >> simply >> comes from the value read from BAR register, which is absolutely a device >> address. >> 5. What we do in patch 3/3 is also to convert the device address in >> PciBar[BarIndex].BaseAddress to host address before returning to the >> caller of >> PciIoGetBarAttributes(). >> >> Please let me know your comments. >> >> For other comments, I have no strong opinion and I can follow the >> conclusion of >> the community. > > > So the issue (13) (not 12) is closed. > But what's your opinion to my comment (1): Explicitly add assertion and > if-check to make sure the alignment meets requirement. > > Ard, > I provided the detailed comments because I felt the code is almost > finalized. > So the detailed comments can avoid creating more versions of patches. > > For the EMPTY comments and STATIC modifier, I created the first version of > this driver, so I'd like to make the coding style in this driver is > consistent. > But if you insist to use a different style, I am fine as long as the coding > style rule allows. > Heyi is spending a lot of time to fix an important limitation in the generic PCI host bridge driver, and so I would like to remain on topic, that's all. And for the comment style: I personally prefer the empty lines before and after as well, but it is not reasonable to reject code that follows the coding style.