On 06/04/2015 04:04 PM, 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. The extra-roots file was existing before PXB. I am afraid to break some other thing. This is why I prefer another file.
> >> 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? While I dont' really care about the type, Pmultiple pci segments correspond to multiple *host bridges*, as opposed to one host bridge with multiple root bridges. Once you support multiple host bridges, pxbs are not really needed. (PCIe based machines) > > 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. OK for 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_n ame_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! Marcel > > Thanks! > Laszlo > ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel