On 20-10-27 15:36:12, Igor Mammedov wrote:
> 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.
I think this will only hit things that subclass TYPE_PCI_HOST_BRIDGE. AFAICT,
ivshm is a regular TYPE_PCI_DEVICE. Is there a _CRS created for ivshm?
>
> PS:
> please use git's --cover-letter option to create multi-patch series,
> in the future
Will do. I wasn't sure what the cutoff was, but the wiki is pretty clear that
"multiple" is the cutoff and it's important to CI.
>
> >
> > ---
> > 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);
> > + }
> > }
> > }
> >
>
>
