From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191])
 by mx.groups.io with SMTP id smtpd.web08.911.1610695531675580211
 for <devel@edk2.groups.io>;
 Thu, 14 Jan 2021 23:25:32 -0800
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: huawei.com, ip: 45.249.212.191, mailfrom: cenjiahui@huawei.com)
Received: from DGGEMS411-HUB.china.huawei.com (unknown [172.30.72.58])
	by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4DHCNT12zmzMKgc;
	Fri, 15 Jan 2021 15:24:09 +0800 (CST)
Received: from [10.174.184.155] (10.174.184.155) by
 DGGEMS411-HUB.china.huawei.com (10.3.19.211) with Microsoft SMTP Server id
 14.3.498.0; Fri, 15 Jan 2021 15:25:23 +0800
Subject: Re: [edk2-devel] [PATCH v4 7/9] OvmfPkg/PciHostBridgeUtilityLib: Extend parameter list of GetRootBridges
To: Laszlo Ersek <lersek@redhat.com>, <devel@edk2.groups.io>
CC: Jordan Justen <jordan.l.justen@intel.com>, Ard Biesheuvel
	<ard.biesheuvel@arm.com>, Rebecca Cran <rebecca@bsdio.com>, Peter Grehan
	<grehan@freebsd.org>, Anthony Perard <anthony.perard@citrix.com>, "Julien
 Grall" <julien@xen.org>, Leif Lindholm <leif@nuviainc.com>, Sami Mujawar
	<sami.mujawar@arm.com>, <xieyingtai@huawei.com>, <wu.wubin@huawei.com>,
	"Yubo Miao" <miaoyubo@huawei.com>
References: <20210112094549.10238-1-cenjiahui@huawei.com>
 <20210112094549.10238-8-cenjiahui@huawei.com>
 <28c4d432-f281-91dc-1b93-faf67297f181@redhat.com>
From: "Jiahui Cen" <cenjiahui@huawei.com>
Message-ID: <9d0979a4-f35a-4114-c8ff-cede015556f1@huawei.com>
Date: Fri, 15 Jan 2021 15:25:20 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.2.2
MIME-Version: 1.0
In-Reply-To: <28c4d432-f281-91dc-1b93-faf67297f181@redhat.com>
X-Originating-IP: [10.174.184.155]
X-CFilter-Loop: Reflected
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit

Hi Laszlo,

On 2021/1/14 18:46, Laszlo Ersek wrote:
>> @@ -124,6 +132,10 @@ PciHostBridgeUtilityGetRootBridges (
>>    UINTN                    *Count,
>>    UINT64                   Attributes,
>>    UINT64                   AllocationAttributes,
>> +  BOOLEAN                  DmaAbove4G,
>> +  BOOLEAN                  NoExtendedConfigSpace,
>> +  UINT32                   BusMin,
>> +  UINT32                   BusMax,
>>    PCI_ROOT_BRIDGE_APERTURE *Io,
>>    PCI_ROOT_BRIDGE_APERTURE *Mem,
>>    PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,
> (1) You forgot to annotate the new params with IN. (Also update the C
> file please, in sync.)
> 
> (2) The BusMin / BusMax addition must absolutely be a separate patch, so
> that we can discuss and review it separately. It's not a simple data
> propagation change -- it generalizes the function internally.
> 
> (3) BusMax should be documented as an inclusive maximum (but see more on
> this below).

A little bit confused. IIUC, the original hard-coded bus ranges, from 0 to
PCI_MAX_BUS, are inclusive, as PCI_MAX_BUS = 255. So in my opinion, the
addition of BusMin/BusMax simply extends the parameters, like DmaAbove4G
and NoExtendedConfigSpace, and replaces [0, PCI_MAX_BUS] with [BusMin, BusMax].
Please correct me if I misunderstand.

Does it really generalize the function?

Thanks,
Jiahui