Re: [Qemu-devel] [PATCH v4 for-2.3 13/25] hw/acpi: remove from root bus 0 the crs resources used by other busses.

[Marcel Apfelbaum <marcel@redhat.com>]

On 03/08/2015 06:13 PM, Kevin O'Connor wrote:
> On Sun, Mar 08, 2015 at 01:16:15PM +0200, Marcel Apfelbaum wrote:
>> If multiple root busses are used, root bus 0 cannot use all the
>> pci holes ranges. Remove the IO/mem ranges used by the other
>> primary busses.
> [...]
>> -    aml_append(crs,
>> -        aml_word_io(aml_min_fixed, aml_max_fixed,
>> -                    aml_pos_decode, aml_entire_range,
>> -                    0x0000, 0x0D00, 0xFFFF, 0x0000, 0xF300));
>> +
>> +    /* prepare PCI IO ranges */
>> +    range.base = 0x0D00;
>> +    range.limit = 0xFFFF;
>> +    if (QLIST_EMPTY(&io_ranges)) {
>> +        aml_append(crs,
>> +            aml_word_io(aml_min_fixed, aml_max_fixed,
>> +                        aml_pos_decode, aml_entire_range,
>> +                        0x0000, range.base, range.limit,
>> +                        0x0000, range.limit - range.base + 1));
>> +    } else {
>> +        QLIST_FOREACH(entry, &io_ranges, entry) {
>> +            if (range.base < entry->base) {
>> +                aml_append(crs,
>> +                    aml_word_io(aml_min_fixed, aml_max_fixed,
>> +                                aml_pos_decode, aml_entire_range,
>> +                                0x0000, range.base, entry->base - 1,
>> +                                0x0000, entry->base - range.base));
>> +            }
>> +            range.base = entry->limit + 1;
>> +            if (!QLIST_NEXT(entry, entry)) {
>> +                aml_append(crs,
>> +                    aml_word_io(aml_min_fixed, aml_max_fixed,
>> +                                aml_pos_decode, aml_entire_range,
>> +                                0x0000, range.base, range.limit,
>> +                                0x0000, range.limit - range.base + 1));
>> +            }
>> +        }
>> +    }
>

Hi Kevin,
Thank you for your review!

> If I read this correctly, it looks like a machine with two root buses
> and 20 devices, each with one memory range and one io range, would end
> up with 40 CRS ranges (ie, a CRS range for every resource).
Correct.

As Michael pointed out in another thread, the firmware is considered
guest code and QEMU cannot assume anything on how the resources are
assigned. This is why this solution was chosen.

However we have two things that make the situation a little better.
1. The PXB implementation includes a pci-bridge and all devices are automatically
    attached to the secondary bus, in this way we have one IO/MEM range per extra root bus.
2. On top of this series we can add a merge algorithm that will bring together
    consecutive ranges. This series does not include this optimization and it
    focuses on the correctness.

   It also
> looks like this furthers the requirement that the guest firmware
> assign the PCI resources prior to QEMU being able to generate the ACPI
> tables.
>
> Am I correct?  If so, that doesn't sound ideal.
You are correct, however is not that bad because we have the following sequence:
  - Early in the boot sequence the bios scans the PCI buses and assigns IO/MEM ranges
  - At this moment all resources needed by QEMU are present in the configuration space.
  - At the end of the boot sequence the BIOS queries the ACPI tables and *only then*
    the tables are computed.

I think we use that implicitly for other features, anyway, it looks like an elegant
solution with no real drawbacks. (Our assumptions are safe)

Thanks,
Marcel
>
> -Kevin
>