Change in flashrom[master]: pcidev: On arm64 we are not getting a valid BAR0 address
Martijn Berger has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39965 ) Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ...................................................................... pcidev: On arm64 we are not getting a valid BAR0 address It seems that there is some code to work around a possible bug in old versions of libpci. This trusts libpci if it is new enough Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Signed-off-by: Martijn Berger <martijn.berger@gmail.com> --- M pcidev.c M programmer.h 2 files changed, 30 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/39965/1 diff --git a/pcidev.c b/pcidev.c index 54c1fd3..973acde 100644 --- a/pcidev.c +++ b/pcidev.c @@ -30,20 +30,47 @@ TYPE_UNKNOWN }; +int pci_bar_nuber_from_offset(int offset) +{ + switch (offset) { + case PCI_BASE_ADDRESS_0: + return 0; + case PCI_BASE_ADDRESS_1: + return 1; + case PCI_BASE_ADDRESS_2: + return 2; + case PCI_BASE_ADDRESS_3: + return 3; + case PCI_BASE_ADDRESS_4: + return 4; + case PCI_BASE_ADDRESS_5: + return 5; + default: + return -1; + } + return -1; +} + uintptr_t pcidev_readbar(struct pci_dev *dev, int bar) { uint64_t addr; uint32_t upperaddr; uint8_t headertype; uint16_t supported_cycles; + int bar_number; enum pci_bartype bartype = TYPE_UNKNOWN; headertype = pci_read_byte(dev, PCI_HEADER_TYPE) & 0x7f; msg_pspew("PCI header type 0x%02x\n", headertype); + bar_number = pci_bar_nuber_from_offset(bar); - /* Don't use dev->base_addr[x] (as value for 'bar'), won't work on older libpci. */ - addr = pci_read_long(dev, bar); + if (PCI_LIB_VERSION >= 0x030500 && bar_number > -1) { + addr = dev->base_addr[bar_number]; + } else { + /* Don't use dev->base_addr[x] (as value for 'bar'), won't work on older libpci. */ + addr = pci_read_long(dev, bar); + } /* Sanity checks. */ switch (headertype) { diff --git a/programmer.h b/programmer.h index 9a41be1..5d4c1d7 100644 --- a/programmer.h +++ b/programmer.h @@ -190,6 +190,7 @@ // FIXME: This needs to be local, not global(?) extern struct pci_access *pacc; int pci_init_common(void); +int pci_bar_nuber_from_offset(int offset); uintptr_t pcidev_readbar(struct pci_dev *dev, int bar); struct pci_dev *pcidev_init(const struct dev_entry *devs, int bar); /* rpci_write_* are reversible writes. The original PCI config space register -- To view, visit https://review.coreboot.org/c/flashrom/+/39965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Gerrit-Change-Number: 39965 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-MessageType: newchange
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39965 ) Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG@7 PS1, Line 7: pcidev: On arm64 we are not getting a valid BAR0 address The commit summary should be a sentence in imperative tense, which summarizes what this change does. The currrent sentence briefly describes the problem you have encountered. https://review.coreboot.org/c/flashrom/+/39965/1/pcidev.c File pcidev.c: https://review.coreboot.org/c/flashrom/+/39965/1/pcidev.c@33 PS1, Line 33: nuber number -- To view, visit https://review.coreboot.org/c/flashrom/+/39965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Gerrit-Change-Number: 39965 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 31 Mar 2020 13:04:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39965 ) Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ...................................................................... Patch Set 1: (2 comments) Thank you for your fixes! https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG@9 PS1, Line 9: It seems that there is some code to work around a possible bug in : old versions of libpci. Please add the problem description again. Maybe (no idea if correct):
… On arm64, this results in an invalid BAR0 address.
https://review.coreboot.org/c/flashrom/+/39965/1//COMMIT_MSG@10 PS1, Line 10: This trusts libpci if it is new enough Please add a dot/period at the end of the sentence. Please mention, the version in the commit message too. -- To view, visit https://review.coreboot.org/c/flashrom/+/39965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Gerrit-Change-Number: 39965 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 01 Apr 2020 07:10:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/39965 to look at the new patch set (#2). Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ...................................................................... pcidev: On arm64 we are not getting a valid BAR0 address It seems that there is some code to work around a possible bug in old versions of libpci. This trusts libpci if it is new enough Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Signed-off-by: Martijn Berger <martijn.berger@gmail.com> --- M pcidev.c M programmer.h 2 files changed, 30 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/39965/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/39965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Gerrit-Change-Number: 39965 Gerrit-PatchSet: 2 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39965 ) Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ...................................................................... Patch Set 3: (2 comments) https://review.coreboot.org/c/flashrom/+/39965/3/pcidev.c File pcidev.c: https://review.coreboot.org/c/flashrom/+/39965/3/pcidev.c@54 PS3, Line 54: int bar unsigned bar https://review.coreboot.org/c/flashrom/+/39965/3/pcidev.c@66 PS3, Line 66: bar_number = pci_bar_number_from_offset(bar); why not just `int bar_number = bar < 6 ? bar : -1;` here and make bar unsigned? -- To view, visit https://review.coreboot.org/c/flashrom/+/39965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Gerrit-Change-Number: 39965 Gerrit-PatchSet: 3 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Edward O'Callaghan <quasisec@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 22 Apr 2020 01:07:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39965 ) Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ...................................................................... Patch Set 4: Code-Review+1 (3 comments) https://review.coreboot.org/c/flashrom/+/39965/4/pcidev.c File pcidev.c: https://review.coreboot.org/c/flashrom/+/39965/4/pcidev.c@33 PS4, Line 33: int pci_bar_number_from_offset(int offset) I'd make this static https://review.coreboot.org/c/flashrom/+/39965/4/pcidev.c@48 PS4, Line 48: default: : return -1; : } : return -1; One of these return statements is redundant https://review.coreboot.org/c/flashrom/+/39965/4/pcidev.c@60 PS4, Line 60: bar_number bar_index? -- To view, visit https://review.coreboot.org/c/flashrom/+/39965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Gerrit-Change-Number: 39965 Gerrit-PatchSet: 4 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 14 Jul 2020 10:41:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Martijn Berger. Anastasia Klimchuk has posted comments on this change by Martijn Berger. ( https://review.coreboot.org/c/flashrom/+/39965?usp=email ) Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: Martijn, are you still here by any chance? Are you still interested to finish this patch? What the "new enough" version of libpci? in the code it is 0x030500 , does it mean 3.5.0 ? -- To view, visit https://review.coreboot.org/c/flashrom/+/39965?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Gerrit-Change-Number: 39965 Gerrit-PatchSet: 4 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm@chromium.org> Gerrit-CC: Edward O'Callaghan <quasisec@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Attention: Martijn Berger Gerrit-Comment-Date: Sat, 13 Sep 2025 09:24:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
participants (5)
-
Anastasia Klimchuk (Code Review) -
Angel Pons (Code Review)
-
Edward O'Callaghan (Code Review)
-
Martijn Berger (Code Review)
-
Paul Menzel (Code Review)