| Message ID | 1471847752-26574-2-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 08/22/16 02:35, Ard Biesheuvel wrote: > Setting the linux,pci-probe-only was intended to align OSes booting via > DT with OSes booting via ACPI in the way they honor the PCI configuration > performed by the firmware. However, ACPI on arm64 does not currently honor > the firmware's PCI configuration, and the linux,pci-probe-only completely > prevents any PCI reconfiguration from occurring under the OS, including > what is needed to support PCI hotplug. > > Since the primary use case was OS access to the GOP framebuffer (which > breaks when the framebuffer BAR is moved when the OS reconfigures the > PCI), we can undo this change now that ArmVirtQemu has moved to a GOP > implementation that does not expose a raw framebuffer in the first place. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c | 38 -------------------- > 1 file changed, 38 deletions(-) > > diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c > index 5063782bb392..669c90355889 100644 > --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c > +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c > @@ -79,42 +79,6 @@ PCI_HOST_BRIDGE_INSTANCE mPciHostBridgeInstanceTemplate = { > // Implementation > // > > -STATIC > -VOID > -SetLinuxPciProbeOnlyProperty ( > - IN FDT_CLIENT_PROTOCOL *FdtClient > - ) > -{ > - INT32 Node; > - UINT32 Tmp; > - EFI_STATUS Status; > - > - if (!FeaturePcdGet (PcdPureAcpiBoot)) { > - // > - // Set the /chosen/linux,pci-probe-only property to 1, so that the PCI > - // setup we will perform in the firmware is honored by the Linux OS, > - // rather than torn down and done from scratch. This is generally a more > - // sensible approach, and aligns with what ACPI based OSes do typically. > - // > - // In case we are exposing an emulated VGA PCI device to the guest, which > - // may subsequently get exposed via the Graphics Output protocol and > - // driven as an efifb by Linux, we need this setting to prevent the > - // framebuffer from becoming unresponsive. > - // > - Status = FdtClient->GetOrInsertChosenNode (FdtClient, &Node); > - > - if (!EFI_ERROR (Status)) { > - Tmp = SwapBytes32 (1); > - Status = FdtClient->SetNodeProperty (FdtClient, Node, > - "linux,pci-probe-only", &Tmp, sizeof (Tmp)); > - } > - if (EFI_ERROR (Status)) { > - DEBUG ((EFI_D_WARN, > - "Failed to set /chosen/linux,pci-probe-only property\n")); > - } > - } > -} > - > // > // We expect the "ranges" property of "pci-host-ecam-generic" to consist of > // records like this. > @@ -293,8 +257,6 @@ ProcessPciHost ( > // > ASSERT (PcdGet64 (PcdPciExpressBaseAddress) == ConfigBase); > > - SetLinuxPciProbeOnlyProperty (FdtClient); > - > DEBUG ((EFI_D_INFO, "%a: Config[0x%Lx+0x%Lx) Bus[0x%x..0x%x] " > "Io[0x%Lx+0x%Lx)@0x%Lx Mem[0x%Lx+0x%Lx)@0x%Lx\n", __FUNCTION__, ConfigBase, > ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize, *IoTranslation, *MmioBase, > I suggest to note in the commit message that this patch effectively undoes commit 8b816c624dd407e1590d7c399c69ab307af3ef33. Can be done without a resend, if you agree. Reviewed-by: Laszlo Ersek <lersek@redhat.com>0 (Further review feedback will likely be very slow, for the known reasons -- I try to process my email backlog whenever I find a reasonable time slot. For larger patches, larger slots are necessary.) Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 23 August 2016 at 23:23, Laszlo Ersek <lersek@redhat.com> wrote: > On 08/22/16 02:35, Ard Biesheuvel wrote: >> Setting the linux,pci-probe-only was intended to align OSes booting via >> DT with OSes booting via ACPI in the way they honor the PCI configuration >> performed by the firmware. However, ACPI on arm64 does not currently honor >> the firmware's PCI configuration, and the linux,pci-probe-only completely >> prevents any PCI reconfiguration from occurring under the OS, including >> what is needed to support PCI hotplug. >> >> Since the primary use case was OS access to the GOP framebuffer (which >> breaks when the framebuffer BAR is moved when the OS reconfigures the >> PCI), we can undo this change now that ArmVirtQemu has moved to a GOP >> implementation that does not expose a raw framebuffer in the first place. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c | 38 -------------------- >> 1 file changed, 38 deletions(-) >> >> diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c >> index 5063782bb392..669c90355889 100644 >> --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c >> +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c >> @@ -79,42 +79,6 @@ PCI_HOST_BRIDGE_INSTANCE mPciHostBridgeInstanceTemplate = { >> // Implementation >> // >> >> -STATIC >> -VOID >> -SetLinuxPciProbeOnlyProperty ( >> - IN FDT_CLIENT_PROTOCOL *FdtClient >> - ) >> -{ >> - INT32 Node; >> - UINT32 Tmp; >> - EFI_STATUS Status; >> - >> - if (!FeaturePcdGet (PcdPureAcpiBoot)) { >> - // >> - // Set the /chosen/linux,pci-probe-only property to 1, so that the PCI >> - // setup we will perform in the firmware is honored by the Linux OS, >> - // rather than torn down and done from scratch. This is generally a more >> - // sensible approach, and aligns with what ACPI based OSes do typically. >> - // >> - // In case we are exposing an emulated VGA PCI device to the guest, which >> - // may subsequently get exposed via the Graphics Output protocol and >> - // driven as an efifb by Linux, we need this setting to prevent the >> - // framebuffer from becoming unresponsive. >> - // >> - Status = FdtClient->GetOrInsertChosenNode (FdtClient, &Node); >> - >> - if (!EFI_ERROR (Status)) { >> - Tmp = SwapBytes32 (1); >> - Status = FdtClient->SetNodeProperty (FdtClient, Node, >> - "linux,pci-probe-only", &Tmp, sizeof (Tmp)); >> - } >> - if (EFI_ERROR (Status)) { >> - DEBUG ((EFI_D_WARN, >> - "Failed to set /chosen/linux,pci-probe-only property\n")); >> - } >> - } >> -} >> - >> // >> // We expect the "ranges" property of "pci-host-ecam-generic" to consist of >> // records like this. >> @@ -293,8 +257,6 @@ ProcessPciHost ( >> // >> ASSERT (PcdGet64 (PcdPciExpressBaseAddress) == ConfigBase); >> >> - SetLinuxPciProbeOnlyProperty (FdtClient); >> - >> DEBUG ((EFI_D_INFO, "%a: Config[0x%Lx+0x%Lx) Bus[0x%x..0x%x] " >> "Io[0x%Lx+0x%Lx)@0x%Lx Mem[0x%Lx+0x%Lx)@0x%Lx\n", __FUNCTION__, ConfigBase, >> ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize, *IoTranslation, *MmioBase, >> > > I suggest to note in the commit message that this patch effectively > undoes commit 8b816c624dd407e1590d7c399c69ab307af3ef33. Can be done > without a resend, if you agree. > Yes, that makes sense. > Reviewed-by: Laszlo Ersek <lersek@redhat.com>0 > Thanks _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c index 5063782bb392..669c90355889 100644 --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c @@ -79,42 +79,6 @@ PCI_HOST_BRIDGE_INSTANCE mPciHostBridgeInstanceTemplate = { // Implementation // -STATIC -VOID -SetLinuxPciProbeOnlyProperty ( - IN FDT_CLIENT_PROTOCOL *FdtClient - ) -{ - INT32 Node; - UINT32 Tmp; - EFI_STATUS Status; - - if (!FeaturePcdGet (PcdPureAcpiBoot)) { - // - // Set the /chosen/linux,pci-probe-only property to 1, so that the PCI - // setup we will perform in the firmware is honored by the Linux OS, - // rather than torn down and done from scratch. This is generally a more - // sensible approach, and aligns with what ACPI based OSes do typically. - // - // In case we are exposing an emulated VGA PCI device to the guest, which - // may subsequently get exposed via the Graphics Output protocol and - // driven as an efifb by Linux, we need this setting to prevent the - // framebuffer from becoming unresponsive. - // - Status = FdtClient->GetOrInsertChosenNode (FdtClient, &Node); - - if (!EFI_ERROR (Status)) { - Tmp = SwapBytes32 (1); - Status = FdtClient->SetNodeProperty (FdtClient, Node, - "linux,pci-probe-only", &Tmp, sizeof (Tmp)); - } - if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_WARN, - "Failed to set /chosen/linux,pci-probe-only property\n")); - } - } -} - // // We expect the "ranges" property of "pci-host-ecam-generic" to consist of // records like this. @@ -293,8 +257,6 @@ ProcessPciHost ( // ASSERT (PcdGet64 (PcdPciExpressBaseAddress) == ConfigBase); - SetLinuxPciProbeOnlyProperty (FdtClient); - DEBUG ((EFI_D_INFO, "%a: Config[0x%Lx+0x%Lx) Bus[0x%x..0x%x] " "Io[0x%Lx+0x%Lx)@0x%Lx Mem[0x%Lx+0x%Lx)@0x%Lx\n", __FUNCTION__, ConfigBase, ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize, *IoTranslation, *MmioBase,
Setting the linux,pci-probe-only was intended to align OSes booting via DT with OSes booting via ACPI in the way they honor the PCI configuration performed by the firmware. However, ACPI on arm64 does not currently honor the firmware's PCI configuration, and the linux,pci-probe-only completely prevents any PCI reconfiguration from occurring under the OS, including what is needed to support PCI hotplug. Since the primary use case was OS access to the GOP framebuffer (which breaks when the framebuffer BAR is moved when the OS reconfigures the PCI), we can undo this change now that ArmVirtQemu has moved to a GOP implementation that does not expose a raw framebuffer in the first place. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c | 38 -------------------- 1 file changed, 38 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel