Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pci pmem #134

Merged
merged 2 commits into from
Jun 14, 2024
Merged

Pci pmem #134

merged 2 commits into from
Jun 14, 2024

Conversation

krystian-hebel
Copy link
Contributor

No description provided.

@macpijan
Copy link
Contributor

@SergiiDmytruk Can you please take a look if you have approached this area as well when rebasing (updating) edk2?

@SergiiDmytruk
Copy link
Member

SergiiDmytruk commented May 27, 2024

I remember those patches, but I didn't analyze the code and reasons for them much. As I understand:

  • coreboot produces state which EDK can't handle
  • both old (no or very little pmem) and new code (some pmem) are workarounds for that
  • new code makes if-statement in
    //
    // Get the Address Translation Offset
    //
    if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
    Descriptor->AddrTranslationOffset = GetMmioAddressTranslationOffset (
    PciIoDevice->PciRootBridgeIo,
    Descriptor->AddrRangeMin,
    Descriptor->AddrLen
    );
    if (Descriptor->AddrTranslationOffset == (UINT64) -1) {
    FreePool (Descriptor);
    return EFI_UNSUPPORTED;
    }
    }
    // According to UEFI spec 2.7, we need return host address for
    // PciIo->GetBarAttributes, and host address = device address - translation.
    Descriptor->AddrRangeMin -= Descriptor->AddrTranslationOffset;

    to not execute for some region of memory in case of coreboot+EDK in Qemu, because now it remains pmem

If that's correct, then this is an improvement in a sense that things are less broken in this case :) We might want to have this coreboot<->EDK incompatibility tracked somewhere though if it's not already tracked.

@krystian-hebel
Copy link
Contributor Author

Possibly the same thing caused https://ticket.coreboot.org/issues/415 and https://ticket.coreboot.org/issues/499. In QEMU I initially had it the other way around, CONFIG_RESOURCE_ALLOCATION_TOP_DOWN made it work fail elsewhere.

Resource allocator used by coreboot may produce intertwined prefetchable
and non-prefetchable MMIO regions. Since edk2 assumes that there is at
most one continuous region of given type, this may create overlaps.

This change removes overlapping part of region from PMem, leaving it
only in Mem (and similarly for Above4G variants). By doing so, some of
memory regions that could otherwise be WC are now UC, but this is safer
than doing it the other way around.

The regions are not split into smaller ones, as doing so would lead to
bigger fragmentation and potentially depletion of MTRRs.

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
@miczyg1 miczyg1 merged commit 28402d1 into extract_vtd Jun 14, 2024
1 check passed
@miczyg1 miczyg1 deleted the pci_pmem branch June 14, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants