On Mon, 26 Oct 2020 12:39:24 -0700
Ben Widawsky <ben.widaw...@intel.com> wrote:
> According to PCIe spec 5.0 Type 1 header space Base Address Registers
> are defined by 7.5.1.2.1 Base Address Registers (same as Type 0). The
> _CRS region should allow for the same range (up to 64b). Prior to this
> change, any host bridge utilizing more than 32b for the BAR would have
> the address truncated and likely lead to conflicts when the operating
> systems reads the _CRS object.
>
> Signed-off-by: Ben Widawsky <ben.widaw...@intel.com>
Looks good to me, so
Reviewed-by: Igor Mammedov <imamm...@redhat.com>
CCing,
Michael to have a send pair of eyes on it
but I wonder how/why ivshm (which might have quite large BAR) works.
PS:
please use git's --cover-letter option to create multi-patch series,
in the future
>
> ---
> I don't think this effects any code currently in QEMU. You'd need to
> have a host bridge which has a BAR, and that BAR wants to be > 32b. I've
> hit this because I have a modified PXB device that does advertise a 64b
> BAR. Also, you'd need a platform that cares about ACPI, which, many do
> not.
> ---
> hw/i386/acpi-build.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index df13abecf4..75604bdc74 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -789,8 +789,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet
> *range_set)
> crs_range_insert(temp_range_set.io_ranges,
> range_base, range_limit);
> } else { /* "memory" */
> - crs_range_insert(temp_range_set.mem_ranges,
> - range_base, range_limit);
> + uint64_t length = range_limit - range_base + 1;
> + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
> + crs_range_insert(temp_range_set.mem_ranges, range_base,
> + range_limit);
> + } else {
> + crs_range_insert(temp_range_set.mem_64bit_ranges,
> + range_base, range_limit);
> + }
> }
> }
>
