On Thu, Jun 04, 2015 at 03:04:15PM +0200, Laszlo Ersek wrote:
> On 06/04/15 11:42, Marcel Apfelbaum wrote:
> > On 06/04/2015 02:11 AM, Laszlo Ersek wrote:
>
> >> What element type do you propose for the array in the new fw_cfg file?
> >> (And what name for the fw_cfg file itself?)
> >>
> >> "etc/extra-pci-roots" uses uint64_t, little endian, for the number of
> >> extra root buses. (In fact if you expose the explicit list in a separate
> >> file, then the element count is not even necessary separately, because
> >> file sizes are available in the fw_cfg directory, and I can divide the
> >> file size with the element size.)
>
> > I can prepare another file.
>
> As long as we're crossing neither a QEMU nor a SeaBIOS release boundary,
> I think we could just change the contents of the same file, with the
> existing name.
>
> > Regarding the new array, each element
> > should be
> > a number between 0x0 and 0xff, so a uint8_t seems fair.
>
> Hm. The number of bytes to save here is really small, and it has been
> suggested to maybe try to support segments? I don't know anything about
> PCI segments; I vaguely recall that it allows for disjoint bus
> intervals, with each interval having at most 256 elements. Maybe we
> could accommodate that with a uint32_t element type?
For segments, we are going to have MCFG data tables,
so IMO there's no need for special tricks.
This is what we are going to use for pci express, down the road.
> In any case I'll leave it to you. I'll simply make the element type a
> typedef in the OVMF code, and then I can easily flip it to another
> integer type if necessary. One thing we should agree upon though that
> whatever the width, it should be little endian.
>
> >> I have two more questions (raised earlier), about the _HID and the _UIDs
> >> in the SSDT.
> >>
> >> First, I can see in your patch
> >>
> >> hw/acpi: add support for i440fx 'snooping' root busses
> >>
> >> that the _UID is populated for each root bus with a string of the form
> >>
> >> PC%02X
> >>
> >> where the argument is "bus_num". UEFI can accommodate this, with the
> >> Expanded ACPI Device Path node, but I'll have to know if the "bus_num"
> >> argument matches the exact numer that you're going to pass down in the
> >> new fw_cfg file. Does it?
>
> > Yes.
>
> Great, thanks.
>
> >>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >>> index db32fd1..8fae3b9 100644
> >>> --- a/hw/i386/acpi-build.c
> >>> +++ b/hw/i386/acpi-build.c
> >>> @@ -944,9 +944,8 @@ build_ssdt(GArray *table_data, GArray *linker,
> >>>
> >>> scope = aml_scope("\\_SB");
> >>> dev = aml_device("PC%.02X", bus_num);
> >>> - aml_append(dev,
> >>> - aml_name_decl("_UID", aml_string("PC%.02X",
> >>> bus_num)));
> >>> - aml_append(dev, aml_name_decl("_HID",
> >>> aml_string("PNP0A03")));
> >>> + aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
> >>> + aml_append(dev, aml_name_decl("_HID",
> >>> aml_eisaid("PNP0A03")));
> >>> aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
> >>>
> >>> if (numa_node != NUMA_NODE_UNASSIGNED) {
> >>
> >> As far as I can see in the QEMU source, filling in _HID and _UID like
> >> this is existing practice.
>
> > I can submit the patch , (or you can submit and I'll ack) on top of PXB
> > series.
>
> I think I'll apply this locally for now, and test it together with the
> OVMF code I plan to write. One of us can submit it later (I'm unaware of
> any urgency, but I might be wrong).
>
> > I am going to be on PTO, so it will wait a week :)
>
> Works for me. Have a nice vacation. :)
>
> Thanks!
> Laszlo
