From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web09.5611.1610528936858307648 for ; Wed, 13 Jan 2021 01:08:57 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=HwlazOPO; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1610528936; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=m19nhKe21VZkvDGB480UZ37Hi/iblpUlOySZcXoO3rU=; b=HwlazOPOA103TfY0+ImKzO64ZUp2dmO1g+9aF1YhhcdzqBt1G1pboue4OV7j21FLDKHDbN 1tE/kVjmmoRCFo615xkezYDHLtcthY7OEtTGSxPVw0UXRLA0yDNAgVxIfQeNYyhtNiU5j2 eGdKtCPZTYbDQpXHSRMn88DItDyH7ZA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-433-AWJq1md0Pu2cgsFDOg62Eg-1; Wed, 13 Jan 2021 04:08:52 -0500 X-MC-Unique: AWJq1md0Pu2cgsFDOg62Eg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 494631005D44; Wed, 13 Jan 2021 09:08:51 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-238.ams2.redhat.com [10.36.112.238]) by smtp.corp.redhat.com (Postfix) with ESMTP id 02E787047C; Wed, 13 Jan 2021 09:08:49 +0000 (UTC) Subject: Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability To: "Luo, Heng" , "Ni, Ray" , "Wu, Hao A" , "devel@edk2.groups.io" Cc: "Kinney, Michael D" References: <20210104065954.3901-1-heng.luo@intel.com> <20210104065954.3901-2-heng.luo@intel.com> <9a081622-94c8-d921-1631-b595f189c807@redhat.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 13 Jan 2021 10:08:49 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 01/13/21 07:15, Luo, Heng wrote: > Hi Hao, > Please hold on merging patch now, we are still waiting for some inputs, I will let you know when we reach agreement. I disagree with waiting. The original patch caused a regression. The currently pending patch fixes the regression. Any further input ("agreement") should be processed *after* we have mitigated the regression. The tree is currently in a wrong state. The fix has been reviewed. Hao, please proceed with merging the fix as soon as you can. Thanks Laszlo > > Thanks, > Heng > >> -----Original Message----- >> From: Ni, Ray >> Sent: Wednesday, January 13, 2021 2:07 PM >> To: Wu, Hao A ; Laszlo Ersek ; >> devel@edk2.groups.io; Luo, Heng >> Cc: Kinney, Michael D >> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: >> Support PCIe Resizable BAR Capability >> >> I've given R-b to the two patches. No comments from my side. >> >>> -----Original Message----- >>> From: Wu, Hao A >>> Sent: Wednesday, January 13, 2021 2:00 PM >>> To: Ni, Ray ; Laszlo Ersek ; >>> devel@edk2.groups.io; Luo, Heng >>> Cc: Kinney, Michael D >>> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: >>> Support PCIe Resizable BAR Capability >>> >>>> -----Original Message----- >>>> From: Ni, Ray >>>> Sent: Tuesday, January 12, 2021 10:28 AM >>>> To: Laszlo Ersek ; devel@edk2.groups.io; Luo, >>>> Heng >>>> Cc: Wu, Hao A ; Kinney, Michael D >>>> >>>> Subject: RE: [edk2-devel] [Patch V3 2/2] >> MdeModulePkg/Bus/Pci/PciBusDxe: >>>> Support PCIe Resizable BAR Capability >>>> >>>>>> It seems like the max BAR size is selected first, but if there's >>>>>> a "resource conflict" (running out of a particular resource type >>>>>> aperture), then the minimum BAR size is selected. I don't know >>>>>> what set of devices and/or resizable BARs this logic applies to, >>>>>> if there are multiple of them. >>>> >>>>>> Per the PCIe specification (revision 5.0, version 0.9) 7.8.6: >>>>>> >>>>>> Software determines, through a proprietary mechanism, what the >>>>>> optimal size is for the resource, and programs that size via the BAR >>>>>> Size field of the Resizable BAR Control register. >>>>>> >>>>>> Furthermore, Table 7-114 defines the Bar Size field of the >>>>>> control register stating: >>>>>> >>>>>> The default value of this field is equal to the default size of the >>>>>> address space that the BAR resource is requesting via the BAR's >>>>>> read-only bits. >>>>>> >>>>>> Therefore the maximum size is not necessarily optimal, nor >>>>>> should the minimum size be considered the default. In fact, >>>>>> [we] tested various handoff BAR sizes for [a particular] GPU and >>>>>> found that Windows didn't like the maximum BAR size. >>>>>> >>>>>> Elsewhere in the discussion [1] the AMD author of the kernel >>>>>> support for resizeable BARs indicates that FPGA devices might >>>>>> implement the REBAR capability as part of their standard PCI >>>>>> wrapper ([our] interpretation), but the BAR usage would be >>>>>> determined by the actual bitstream written to the device, >>>>>> therefore there might be a full bitmask for the BAR sizes supported >> by the device. >>>>>> >>>>>> [1] >>>>>> https://lists.freedesktop.org/archives/dri-devel/2021-January/th >>>>>> read >>>>>> .html >>>>>> >>>>>> It would certainly make sense for the firmware to take REBAR >>>>>> capabilities into account when sizing bridge apertures, but to >>>>>> generically enable extended BAR sizes would make lots of >>>>>> assumptions about the device usage and compatibility. >>>>>> >>>>>> [...] At least for GPUs the expectation would be a default, >>>>>> smaller compatibility size expanding to some representation that >>>>>> allows direct DMA to the entire memory of the card. >>>>> >>>>> So this patch should either be reverted; or minimally, the default >>>>> value of "PcdPcieResizableBarSupport" should be set to FALSE, as >>>>> the policy for BAR sizing doesn't look robust or portable. >>>>> >>>>> >>>>> General request for the future: if you implement some kind of >>>>> policy in core edk2, please at least *document* the policy >>>>> somewhere. It's unacceptable to have to decipher the source code >>>>> for such a possibly impactful change in the core. There is no need >>>>> for a wiki page or an RFC, but a sane bugzilla ticket and a sane commit >> message are required. >>>>> >>>>> (The documentation of the PCD in the "MdeModulePkg.dec" file is >>>>> unsatisfactory too, and the UNI file has not been updated at all.) >>>>> >>>> >>>> >>>> >>>> Your understanding is correct. Original idea is to let platform >>>> supply the >>> policy >>>> about what the optimal BAR size is for each resizable BAR. >>>> The current implementation is a try to avoid asking platform code >>>> for such policy because we thought it's a burden for platform to supply >> the policy data. >>>> >>>> I agree that we set the PCD default value as disabled and after a >>>> period of study, we will understand whether a platform policy is really >> needed. >>> >>> >>> Hello Laszlo and Ray, >>> >>> I saw Heng's patch series to >>> 1) Set the PCD default value to FALSE: >>> https://edk2.groups.io/g/devel/message/70139 >>> 2) Update the UNI file: https://edk2.groups.io/g/devel/message/70140 >>> has got Reviewed-by/Acked-by tags from reviewers. >>> >>> Do you have further comments for the series? >>> If not, I will merge this change in the next 24 hours. >>> >>> Thanks in advance. >>> >>> Best Regards, >>> Hao Wu >>> >>> >>>> >>>> Thanks, >>>> Ray