October 2018 - flashrom-gerrit

Change in flashrom[master]: pci2: add wiki printing support
by Luc Verhaegen (Code Review) 12 Oct '18

12 Oct '18

Luc Verhaegen has uploaded this change for review. ( https://review.coreboot.org/29084 Change subject: pci2: add wiki printing support ...................................................................... pci2: add wiki printing support Change-Id: I8c839feda2ef3ec97183f1e06712881786622ebf Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M print.c M print_wiki.c 2 files changed, 100 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/29084/1 diff --git a/print.c b/print.c index 4636e1c..c3e151a 100644 --- a/print.c +++ b/print.c @@ -492,8 +492,7 @@ pci_init(pacc); pci_load_name_list(pacc); - } else - msg_ginfo("Already initialized\n"); + } msg_ginfo("\nSupported PCI devices for the %s programmer:\n", prog.name); diff --git a/print_wiki.c b/print_wiki.c index ea8c4d7..4088dc7 100644 --- a/print_wiki.c +++ b/print_wiki.c @@ -19,6 +19,11 @@ #include <string.h> #include <stdlib.h> #include <time.h> + +#if NEED_PCI == 1 +#include <pci/pci.h> +#endif + #include "flash.h" #include "flashchips.h" #include "programmer.h" @@ -355,18 +360,28 @@ return count; } +static char *vendor_colour_get(void) +{ + static bool vendor_colour = false; + + /* Alternate colors if the vendor changes. */ + vendor_colour = !vendor_colour; + + if (vendor_colour) + return "eeeeee"; + else + return "dddddd"; +} + static void print_supported_devs_wiki_helper(const struct programmer_entry prog) { int i = 0; - static int c = 0; const struct dev_entry *devs = prog.devs.dev; const unsigned int count = count_supported_devs_wiki(devs); - - /* Alternate colors if the vendor changes. */ - c = !c; + char *colour = vendor_colour_get(); for (i = 0; devs[i].vendor_id != 0; i++) { - printf("|- bgcolor=\"#%s\"\n", (c) ? "eeeeee" : "dddddd"); + printf("|- bgcolor=\"#%s\"\n", colour); if (i == 0) printf("| rowspan=\"%u\" | %s |", count, prog.name); printf("| %s || %s || %04x:%04x || {{%s}}\n", devs[i].vendor_name, devs[i].device_name, @@ -374,6 +389,75 @@ } } +#if NEED_PCI == 1 +static int count_supported_pci_matches_wiki(const struct flashrom_pci_match *matches) +{ + int i; + + for (i = 0; matches[i].vendor_id; i++) + ; + + return i; +} + +/* + * libpci lookup_name(PCI_LOOKUP_DEVICE) is currently not reliable. + * We need to work around this. + */ +static void print_supported_pci_matches_wiki_helper(const struct programmer_entry prog) +{ + const struct flashrom_pci_match *matches = prog.devs.pci_match; + char *colour = vendor_colour_get(); + int i, count = count_supported_pci_matches_wiki(matches); + int vendor_length = 0; + uint16_t vendor_id = 0; + char vendor_buffer[256], *vendor_name; + char device_buffer[256], *device_name; + + /* try to use already initialized pci_access structure */ + if (!pacc) { + pacc = pci_alloc(); + if (!pacc) { + msg_gerr("%s: Failed to allocate pci_access " + "structure.\n", __func__); + return; + } + pci_init(pacc); + + pci_load_name_list(pacc); + } + + for (i = 0; matches[i].vendor_id; i++) { + if (matches[i].vendor_id != vendor_id) { + vendor_name = pci_lookup_name(pacc, vendor_buffer, + sizeof(vendor_buffer), + PCI_LOOKUP_VENDOR, + matches[i].vendor_id); + vendor_length = strlen(vendor_name); + vendor_id = matches[i].vendor_id; + } + + device_name = + pci_lookup_name(pacc, device_buffer, + sizeof(device_buffer), + PCI_LOOKUP_VENDOR | PCI_LOOKUP_DEVICE, + matches[i].vendor_id, + matches[i].device_id); + device_name += vendor_length + 1; + + printf("|- bgcolor=\"#%s\"\n", colour); + if (!i) + printf("| rowspan=\"%u\" | %s |", count, prog.name); + printf("| %s || %s || %04x:%04x || {{%s}}\n", + vendor_name, device_name, + matches[i].vendor_id, matches[i].device_id, + test_state_to_template(matches[i].status)); + } + + /* Don't bother cleaning up pacc, we either reuse it, or exit() */ +} +#endif /* NEED_PCI */ + static void print_supported_devs_wiki() { unsigned int pci_count = 0; @@ -389,6 +473,11 @@ case PCI: pci_count += count_supported_devs_wiki(prog.devs.dev); break; +#if NEED_PCI == 1 + case PCI2: + pci_count += count_supported_pci_matches_wiki(prog.devs.pci_match); + break; +#endif /* NEED_PCI */ case OTHER: default: break; @@ -401,9 +490,12 @@ for (i = 0; i < PROGRAMMER_INVALID; i++) { const struct programmer_entry prog = programmer_table[i]; - if (prog.type == PCI) { + if (prog.type == PCI) print_supported_devs_wiki_helper(prog); - } +#if NEED_PCI == 1 + else if (prog.type == PCI2) + print_supported_pci_matches_wiki_helper(prog); +#endif /* NEED_PCI */ } printf("\n|}\n\n|}\n"); -- To view, visit https://review.coreboot.org/29084 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8c839feda2ef3ec97183f1e06712881786622ebf Gerrit-Change-Number: 29084 Gerrit-PatchSet: 1 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be>

1 0

Change in flashrom[master]: pci2: add device list printing support
by Luc Verhaegen (Code Review) 12 Oct '18

12 Oct '18

Luc Verhaegen has uploaded this change for review. ( https://review.coreboot.org/29083 Change subject: pci2: add device list printing support ...................................................................... pci2: add device list printing support Change-Id: I3eea9edfcdf2ae1c35f4c935dca97fa93eaded58 Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M flashrom.c M print.c M programmer.h 3 files changed, 76 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/29083/1 diff --git a/flashrom.c b/flashrom.c index 59a7531..3b3e9bb 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2098,6 +2098,7 @@ switch (p.type) { case USB: case PCI: + case PCI2: case OTHER: if (p.devs.note == NULL) { if (strcmp("internal", p.name) == 0) diff --git a/print.c b/print.c index 30f06c8..4636e1c 100644 --- a/print.c +++ b/print.c @@ -19,6 +19,11 @@ #include <stdio.h> #include <string.h> #include <stdlib.h> + +#if NEED_PCI == 1 +#include <pci/pci.h> +#endif + #include "flash.h" #include "programmer.h" @@ -462,6 +467,62 @@ } } +#if NEED_PCI == 1 +/* + * We depend on libpcis built in list of names here. This makes formatting + * tricky/ugly. + * libpcis device only name lookup is currently not reliable, so look up the + * whole tuple. + */ +void print_supported_flashrom_pci_devices(const struct programmer_entry prog) +{ +#define PAD_LENGTH 64 + const struct flashrom_pci_match *matches = prog.devs.pci_match; + char buffer[256], *name; + unsigned int i, j, len; + + /* try to use already initialized pci_access structure */ + if (!pacc) { + pacc = pci_alloc(); + if (!pacc) { + msg_gerr("%s: Failed to allocate pci_access " + "structure.\n", __func__); + return; + } + pci_init(pacc); + + pci_load_name_list(pacc); + } else + msg_ginfo("Already initialized\n"); + + msg_ginfo("\nSupported PCI devices for the %s programmer:\n", + prog.name); + + len = msg_ginfo("Vendor - Device"); + if (len < PAD_LENGTH) + for (j = 0; j < (PAD_LENGTH - len); j++) + msg_ginfo(" "); + msg_ginfo("\tPCI IDS Status\n"); + + for (i = 0; matches[i].vendor_id; i++) { + name = pci_lookup_name(pacc, buffer, sizeof(buffer), + PCI_LOOKUP_VENDOR | PCI_LOOKUP_DEVICE, + matches[i].vendor_id, + matches[i].device_id); + len = msg_pinfo("%s", name); + if (len < PAD_LENGTH) + for (j = 0; j < (PAD_LENGTH - len); j++) + msg_pinfo(" "); + msg_pinfo("\t%04x:%04x %s\n", + matches[i].vendor_id, matches[i].device_id, + test_state_to_text(matches[i].status)); + } + + /* Don't bother cleaning up pacc, we either reuse it, or exit() */ +} +#endif /* NEED_PCI */ + + int print_supported(void) { unsigned int i; @@ -489,6 +550,11 @@ case PCI: print_supported_devs(prog, "PCI"); break; +#if NEED_PCI == 1 + case PCI2: + print_supported_flashrom_pci_devices(prog); + break; +#endif /* NEED_PCI */ case OTHER: if (prog.devs.note != NULL) { msg_ginfo("\nSupported devices for the %s programmer:\n", prog.name); diff --git a/programmer.h b/programmer.h index 5f02c6e..a5a1360 100644 --- a/programmer.h +++ b/programmer.h @@ -123,6 +123,7 @@ enum programmer_type { PCI = 1, /* to detect uninitialized values */ + PCI2, /* different pci infrastructure */ USB, OTHER, }; @@ -135,12 +136,20 @@ const char *device_name; }; +struct flashrom_pci_match { + uint16_t vendor_id; + uint16_t device_id; + const enum test_state status; + const void *private; /* programmer specific */ +}; + struct programmer_entry { const char *name; const enum programmer_type type; union { const struct dev_entry *const dev; const char *const note; + const struct flashrom_pci_match *pci_match; } devs; int (*init) (void); @@ -850,13 +859,6 @@ struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num); #if NEED_PCI == 1 -struct flashrom_pci_match { - uint16_t vendor_id; - uint16_t device_id; - const enum test_state status; - const void *private; /* programmer specific */ -}; - struct flashrom_pci_device { char *name; /* created from pci info */ -- To view, visit https://review.coreboot.org/29083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3eea9edfcdf2ae1c35f4c935dca97fa93eaded58 Gerrit-Change-Number: 29083 Gerrit-PatchSet: 1 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be>

1 0

Change in flashrom[master]: pci2: add a second device private called data
by Luc Verhaegen (Code Review) 12 Oct '18

12 Oct '18

Luc Verhaegen has uploaded this change for review. ( https://review.coreboot.org/29082 Change subject: pci2: add a second device private called data ...................................................................... pci2: add a second device private called data This needs to be dynamically allocated so it can be used to, for instance, save/restore register values. Change-Id: I7927773a88439ca3828d694785e58a4cf2b7795c Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M pcidev.c M programmer.h 2 files changed, 9 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/29082/1 diff --git a/pcidev.c b/pcidev.c index 7680e78..18ddb3b 100644 --- a/pcidev.c +++ b/pcidev.c @@ -547,10 +547,15 @@ free(device->sysfs_path); device->sysfs_path = NULL; + if (device->private_data) + msg_perr("%s: device \"%s\"still has private data attached!\n" + "Please report a bug at flashrom(a)flashrom.org\n", + device->name, __func__); + device->private = NULL; + free(device->name); device->name = NULL; - device->private = NULL; free(device); return 0; diff --git a/programmer.h b/programmer.h index 7ec4291..5f02c6e 100644 --- a/programmer.h +++ b/programmer.h @@ -871,7 +871,9 @@ volatile uint8_t *mmio; /* mapped io memory */ size_t mmio_size; - const void *private; /* programmer specific hook */ + /* programmer specifics */ + const void *private; /* filled in from flashrom_pci_match */ + void *private_data; /* alloced and freed by programmer */ /* housekeeping */ bool enabled; -- To view, visit https://review.coreboot.org/29082 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7927773a88439ca3828d694785e58a4cf2b7795c Gerrit-Change-Number: 29082 Gerrit-PatchSet: 1 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be>

1 0

Change in flashrom[master]: pci2: add device enable/disable
by Luc Verhaegen (Code Review) 12 Oct '18

12 Oct '18

Luc Verhaegen has uploaded this change for review. ( https://review.coreboot.org/29081 Change subject: pci2: add device enable/disable ...................................................................... pci2: add device enable/disable Change-Id: I79bbc5c4b0f571df692e0992f43b46da7ab3bf01 Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M pcidev.c M programmer.h 2 files changed, 120 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/29081/1 diff --git a/pcidev.c b/pcidev.c index da4e2a3..7680e78 100644 --- a/pcidev.c +++ b/pcidev.c @@ -420,6 +420,115 @@ /* * */ +int +flashrom_pci_device_enable(struct flashrom_pci_device *device) +{ + char filename[1024]; + int fd, ret; + char enable; + + if (device->enabled) + return 0; + + snprintf(filename, sizeof(filename) - 1, + "%senable", device->sysfs_path); + + fd = open(filename, O_RDWR); + if (fd == -1) { + msg_perr("%s: failed to open %s: %s\n", + __func__, filename, strerror(errno)); + return errno; + } + + ret = read(fd, &enable, 1); + if (ret != 1) { + close(fd); + msg_perr("%s: failed to read %s: %s\n", + __func__, filename, strerror(errno)); + return errno; + } + + if (enable == '0') { + device->was_disabled = true; + ret = write(fd, "1", 1); + if (ret != 1) { + close(fd); + msg_perr("%s: failed to write %s: %s\n", + __func__, filename, strerror(errno)); + return errno; + } + } else if (enable != '1') { + close(fd); + msg_perr("%s: invalid value read from %s: %c\n", + __func__, filename, enable); + return EINVAL; + } + + close(fd); + + device->enabled = true; + + return 0; +} + +/* + * + */ +int +flashrom_pci_device_disable(struct flashrom_pci_device *device) +{ + char filename[1024]; + int fd, ret; + char enable; + + /* We do not want to disable the device if we did not enable it */ + if (!device->enabled || !device->was_disabled) + return 0; + + snprintf(filename, sizeof(filename) - 1, + "%senable", device->sysfs_path); + + fd = open(filename, O_RDWR); + if (fd == -1) { + msg_perr("%s: failed to open %s: %s\n", + __func__, filename, strerror(errno)); + return errno; + } + + ret = read(fd, &enable, 1); + if (ret != 1) { + close(fd); + msg_perr("%s: failed to read %s: %s\n", + __func__, filename, strerror(errno)); + return errno; + } + + if (enable == '1') { + ret = write(fd, "0", 1); + if (ret != 1) { + close(fd); + msg_perr("%s: failed to write %s: %s\n", + __func__, filename, strerror(errno)); + return errno; + } + } else if (enable != '0') { + close(fd); + msg_perr("%s: invalid value read from %s: %c\n", + __func__, filename, enable); + return EINVAL; + } + + close(fd); + + device->enabled = false; + + return 0; +} + + +/* + * + */ static int flashrom_pci_device_shutdown(void *data) { @@ -433,6 +542,8 @@ flashrom_pci_mmio_unmap(device); + flashrom_pci_device_disable(device); + free(device->sysfs_path); device->sysfs_path = NULL; @@ -533,6 +644,9 @@ register_shutdown(flashrom_pci_device_shutdown, device); + if (flashrom_pci_device_enable(device)) + return NULL; + return device; } diff --git a/programmer.h b/programmer.h index 1a7af7a..7ec4291 100644 --- a/programmer.h +++ b/programmer.h @@ -872,9 +872,15 @@ size_t mmio_size; const void *private; /* programmer specific hook */ + + /* housekeeping */ + bool enabled; + bool was_disabled; }; struct flashrom_pci_device *flashrom_pci_init(const struct flashrom_pci_match *matches); +int flashrom_pci_device_enable(struct flashrom_pci_device *device); +int flashrom_pci_device_disable(struct flashrom_pci_device *device); int flashrom_pci_mmio_map(struct flashrom_pci_device *device, int bar); void flashrom_pci_mmio_unmap(struct flashrom_pci_device *device); -- To view, visit https://review.coreboot.org/29081 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I79bbc5c4b0f571df692e0992f43b46da7ab3bf01 Gerrit-Change-Number: 29081 Gerrit-PatchSet: 1 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be>

1 0

Change in flashrom[master]: pci2: add read/write/mask byte/long mmio accessors
by Luc Verhaegen (Code Review) 12 Oct '18

12 Oct '18

Luc Verhaegen has uploaded this change for review. ( https://review.coreboot.org/29080 Change subject: pci2: add read/write/mask byte/long mmio accessors ...................................................................... pci2: add read/write/mask byte/long mmio accessors As for the mask command: For those "embedded developers" who grew up with the awkward x86 in/out assembly instructions with the illogical ordering it will seem alien. As a graphics driver developer who has had to deal with thousands of registers at a time, mostly setting a few bits at a time, i found the increased readability and debuggability of a mask command invaluable. Once the ati_spi code is added, it will be clear just how effective this command is. As for the lack of automatic rollback: - it makes little sense to roll back every single register written. One could imagine a scenario where this would perfectly undo the flash being written. - it makes little sense to roll back every mask command: Multiple mask commands are likely to touch the same register. - engines, even simple spi engines, are triggered by a single bit or a single register being written. Automatic rollback would trigger engines before the fitting registers are written. - reading might also trigger an engine. How does one roll back those? How, if the register is automatically read before it is written, are we dealing with that? Instead, by providing programmer internal save/restore functions, the programmer gets full control over what is saved and restored, and the order in which those happen. The fact that we automatically retain the device match private, should make it easier to have device specific save/restore callbacks. Change-Id: I5c3f57e9c510337bb81be30aa6c143840a349a60 Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M pcidev.c M programmer.h 2 files changed, 101 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/29080/1 diff --git a/pcidev.c b/pcidev.c index 8503e78..da4e2a3 100644 --- a/pcidev.c +++ b/pcidev.c @@ -535,3 +535,96 @@ return device; } + +/* + * + * Accessors of mmio. + * + * It is nonsensical to roll these back automatically. + * Programmers are responsible for their own restauration. + * + */ +uint8_t flashrom_pci_mmio_byte_read(struct flashrom_pci_device *device, + off_t address) +{ + if (address >= device->mmio_size) { + errno = EFAULT; + return -1; + } + + return device->mmio[address]; +} + +void flashrom_pci_mmio_byte_write(struct flashrom_pci_device *device, + off_t address, uint8_t value) +{ + if (address >= device->mmio_size) { + errno = EFAULT; + return; + } + + device->mmio[address] = value; +} + +void flashrom_pci_mmio_byte_mask(struct flashrom_pci_device *device, + off_t address, uint8_t value, uint8_t mask) +{ + uint8_t temp; + + if (address >= device->mmio_size) { + errno = EFAULT; + return; + } + + value &= mask; + + temp = device->mmio[address] & ~mask; + temp |= value & mask; + + device->mmio[address] = temp; +} + +uint32_t flashrom_pci_mmio_long_read(struct flashrom_pci_device *device, + off_t address) +{ + volatile uint32_t *mmio = (volatile uint32_t *) &device->mmio[address]; + + if ((address >= device->mmio_size) || (address & 0x03)) { + errno = EFAULT; + return -1; + } + + return mmio[0]; +} + +void flashrom_pci_mmio_long_write(struct flashrom_pci_device *device, + off_t address, uint32_t value) +{ + volatile uint32_t *mmio = (volatile uint32_t *) &device->mmio[address]; + + if ((address >= device->mmio_size) || (address & 0x03)) { + errno = EFAULT; + return; + } + + mmio[0] = value; +} + +void flashrom_pci_mmio_long_mask(struct flashrom_pci_device *device, + off_t address, uint32_t value, uint32_t mask) +{ + volatile uint32_t *mmio = (volatile uint32_t *) &device->mmio[address]; + uint32_t temp; + + if ((address >= device->mmio_size) || (address & 0x03)) { + errno = EFAULT; + return; + } + + value &= mask; + + temp = mmio[0] & ~mask; + temp |= value & mask; + + mmio[0] = temp; +} diff --git a/programmer.h b/programmer.h index d553317..1a7af7a 100644 --- a/programmer.h +++ b/programmer.h @@ -877,6 +877,14 @@ struct flashrom_pci_device *flashrom_pci_init(const struct flashrom_pci_match *matches); int flashrom_pci_mmio_map(struct flashrom_pci_device *device, int bar); void flashrom_pci_mmio_unmap(struct flashrom_pci_device *device); + +uint8_t flashrom_pci_mmio_byte_read(struct flashrom_pci_device *device, off_t address); +void flashrom_pci_mmio_byte_write(struct flashrom_pci_device *device, off_t address, uint8_t value); +void flashrom_pci_mmio_byte_mask(struct flashrom_pci_device *device, off_t address, uint8_t value, uint8_t mask); + +uint32_t flashrom_pci_mmio_long_read(struct flashrom_pci_device *device, off_t address); +void flashrom_pci_mmio_long_write(struct flashrom_pci_device *device, off_t address, uint32_t value); +void flashrom_pci_mmio_long_mask(struct flashrom_pci_device *device, off_t address, uint32_t value, uint32_t mask); #endif /* NEED_PCI */ #endif /* !__PROGRAMMER_H__ */ -- To view, visit https://review.coreboot.org/29080 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5c3f57e9c510337bb81be30aa6c143840a349a60 Gerrit-Change-Number: 29080 Gerrit-PatchSet: 1 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be>

1 0

Change in flashrom[master]: pci2: add mmio bar map/unmap
by Luc Verhaegen (Code Review) 12 Oct '18

12 Oct '18

Luc Verhaegen has uploaded this change for review. ( https://review.coreboot.org/29079 Change subject: pci2: add mmio bar map/unmap ...................................................................... pci2: add mmio bar map/unmap This works through linux sysfs and opens /sys/bus/pci/devices/.../resource%d and maps that to neatly circumvent /dev/mem security limitations and we do not need to figure out BAR address and size ourselves. Change-Id: I1c67d51d853f751d13271bf83316ee2118c1c476 Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M pcidev.c M programmer.h 2 files changed, 75 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/29079/1 diff --git a/pcidev.c b/pcidev.c index 837b028..8503e78 100644 --- a/pcidev.c +++ b/pcidev.c @@ -18,6 +18,11 @@ #include <stdlib.h> #include <string.h> +#include <errno.h> +#include <fcntl.h> +#include <sys/mman.h> +#include <unistd.h> + #include "flash.h" #include "programmer.h" #include "hwaccess.h" @@ -352,6 +357,69 @@ /* * */ +int +flashrom_pci_mmio_map(struct flashrom_pci_device *device, int bar) +{ + char filename[1024]; + int fd; + + if ((bar < 0) || (bar > 5)) { + msg_perr("%s: Invalid BAR provided: %d\n", __func__, bar); + return EINVAL; + } + + if (device->mmio) { + if (device->pci->size[bar] != device->mmio_size) { + msg_perr("%s: already mapped bar: 0x%lXbytes @ %p)\n", + __func__, device->mmio_size, device->mmio); + return EALREADY; + } else + return 0; + } + + snprintf(filename, sizeof(filename) - 1, + "%sresource%d", device->sysfs_path, bar); + + fd = open(filename, O_RDWR); + if (fd == -1) { + msg_perr("%s: failed to open %s: %s\n", + __func__, filename, strerror(errno)); + return errno; + } + + device->mmio_size = device->pci->size[bar]; + + device->mmio = mmap(NULL, device->mmio_size, PROT_WRITE | PROT_READ, + MAP_SHARED, fd, 0); + if (device->mmio == MAP_FAILED) { + msg_perr("%s: mapping %s failed: %s\n", + __func__, filename, strerror(errno)); + close(fd); + device->mmio_size = 0; + return errno; + } + + close(fd); + + return 0; +} + +/* + * + */ +void +flashrom_pci_mmio_unmap(struct flashrom_pci_device *device) +{ + if (device->mmio && device->mmio_size && + (device->mmio != MAP_FAILED)) + munmap((void *) device->mmio, device->mmio_size); + device->mmio = NULL; + device->mmio_size = 0; +} + +/* + * + */ static int flashrom_pci_device_shutdown(void *data) { @@ -363,6 +431,8 @@ return 1; } + flashrom_pci_mmio_unmap(device); + free(device->sysfs_path); device->sysfs_path = NULL; diff --git a/programmer.h b/programmer.h index 0a445fa..d553317 100644 --- a/programmer.h +++ b/programmer.h @@ -868,10 +868,15 @@ char *sysfs_path; /* linux only, of course */ + volatile uint8_t *mmio; /* mapped io memory */ + size_t mmio_size; + const void *private; /* programmer specific hook */ }; struct flashrom_pci_device *flashrom_pci_init(const struct flashrom_pci_match *matches); +int flashrom_pci_mmio_map(struct flashrom_pci_device *device, int bar); +void flashrom_pci_mmio_unmap(struct flashrom_pci_device *device); #endif /* NEED_PCI */ #endif /* !__PROGRAMMER_H__ */ -- To view, visit https://review.coreboot.org/29079 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1c67d51d853f751d13271bf83316ee2118c1c476 Gerrit-Change-Number: 29079 Gerrit-PatchSet: 1 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be>

1 0

Change in flashrom[master]: add newer pci infrastructure
by Luc Verhaegen (Code Review) 12 Oct '18

12 Oct '18

Luc Verhaegen has uploaded this change for review. ( https://review.coreboot.org/29078 Change subject: add newer pci infrastructure ...................................................................... add newer pci infrastructure This code does not negatively affect existing pci support and existing pci devices, it introduces parallel infrastructure which: - allows users to define a private which could be device specific. This avoids doing multiple lookups, which massively helps the ati spi driver which will support hundreds of devices. - uses linux sysfs pci infrastructure for enable/disable and mapping resources. This circumvents the security restrictions surrounding /dev/mem, but means that support for other operating systems still needs to be cobbled together. - looks up device names through libpci, keeping us from maintaining a separate names list. Change-Id: I59af37cba5cb78014e0714c654db2501151f605e Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M pcidev.c M programmer.h 2 files changed, 157 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/29078/1 diff --git a/pcidev.c b/pcidev.c index f5ad819..837b028 100644 --- a/pcidev.c +++ b/pcidev.c @@ -3,6 +3,7 @@ * * Copyright (C) 2009 Uwe Hermann <uwe(a)hermann-uwe.de> * Copyright (C) 2010, 2011 Carl-Daniel Hailfinger + * Copyright (C) 2018 Luc Verhaegen <libv(a)skynet.be> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -333,3 +334,134 @@ register_undo_pci_write_long(dev, reg); return pci_write_long(dev, reg, data); } + +/* + * This is reinvented pci device matching and access infrastructure which: + * - allows users to define a private which could be device specific. + * This avoids doing multiple lookups, which massively helps the ati spi + * driver which will support hundreds of devices. + * - uses linux sysfs pci infrastructure for enable/disable and mapping + * resources. This circumvents the security restrictions surrounding + * /dev/mem, but means that support for other operating systems still + * needs to be cobbled together. + * - looks up device names through libpci, keeping us from maintaining a + * separate names list. + * + */ + +/* + * + */ +static int +flashrom_pci_device_shutdown(void *data) +{ + struct flashrom_pci_device *device = data; + + if (!device->sysfs_path) { + msg_perr("%s: Tried to cleanup an invalid pci_device!\n" + "Please report a bug at flashrom(a)flashrom.org\n", __func__); + return 1; + } + + free(device->sysfs_path); + device->sysfs_path = NULL; + + free(device->name); + device->name = NULL; + + device->private = NULL; + free(device); + + return 0; +} + +/* + * + */ +struct flashrom_pci_device * +flashrom_pci_init(const struct flashrom_pci_match *matches) +{ + struct flashrom_pci_device *device; + struct pci_filter filter; + struct pci_dev *dev, *found_dev = NULL; + const struct flashrom_pci_match *found_match = NULL; + char *pcidev_bdf; + char buffer[1024], *name = NULL; + int i, found = 0; + + if (pci_init_common()) + return NULL; + + pci_filter_init(pacc, &filter); + + /* Filter by bb:dd.f (if supplied by the user). */ + pcidev_bdf = extract_programmer_param("pci"); + if (pcidev_bdf) { + char *msg; + + if ((msg = pci_filter_parse_slot(&filter, pcidev_bdf))) { + msg_perr("Error: %s\n", msg); + return NULL; + } + + free(pcidev_bdf); + } + + for (dev = pacc->devices; dev; dev = dev->next) + if (pci_filter_match(&filter, dev)) { + for (i = 0; matches[i].vendor_id ; i++) + if ((dev->vendor_id == matches[i].vendor_id) && + (dev->device_id == matches[i].device_id)) + break; + + if (!matches[i].vendor_id) + continue; + + + name = pci_lookup_name(pacc, buffer, sizeof(buffer), + PCI_LOOKUP_VENDOR | PCI_LOOKUP_DEVICE, + dev->vendor_id, dev->device_id); + if (name) + msg_pinfo("Detected %04x:%04x@%02x:%02x.%x \"%s\"\n", + dev->vendor_id, dev->device_id, + dev->bus, dev->dev, dev->func, name); + else + msg_pinfo("Detected %04x:%04x@%02x:%02x.%x \"%s\"\n", + dev->vendor_id, dev->device_id, + dev->bus, dev->dev, dev->func, + "<unknown pciids>"); + found_dev = dev; + found_match = &matches[i]; + found++; + } + + /* Only continue if exactly one supported PCI dev has been found. */ + if (!found) { + msg_perr("Error: No supported PCI device found.\n"); + return NULL; + } else if (found > 1) { + msg_perr("Error: Multiple supported PCI devices found. Use 'flashrom -p xxxx:pci=bb:dd.f'\n" + "to explicitly select the card with the given BDF (PCI bus, device, function).\n"); + return NULL; + } + + device = calloc(1, sizeof(*device)); + + device->name = strdup(name); + + device->device_id = found_dev->device_id; + device->vendor_id = found_dev->vendor_id; + + device->pci = found_dev; + + snprintf(buffer, sizeof(buffer) - 1, + "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/", + 0, found_dev->bus, found_dev->dev, found_dev->func); + device->sysfs_path = strdup(buffer); + + device->private = found_match->private; + + register_shutdown(flashrom_pci_device_shutdown, device); + + return device; +} diff --git a/programmer.h b/programmer.h index 311992a..0a445fa 100644 --- a/programmer.h +++ b/programmer.h @@ -849,4 +849,29 @@ struct libusb_device_handle *usb_dev_get_by_vid_pid_number( struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num); +#if NEED_PCI == 1 +struct flashrom_pci_match { + uint16_t vendor_id; + uint16_t device_id; + const enum test_state status; + const void *private; /* programmer specific */ +}; + +struct flashrom_pci_device { + char *name; /* created from pci info */ + + /* convenience copies */ + uint16_t vendor_id; + uint16_t device_id; + + struct pci_dev *pci; + + char *sysfs_path; /* linux only, of course */ + + const void *private; /* programmer specific hook */ +}; + +struct flashrom_pci_device *flashrom_pci_init(const struct flashrom_pci_match *matches); +#endif /* NEED_PCI */ + #endif /* !__PROGRAMMER_H__ */ -- To view, visit https://review.coreboot.org/29078 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I59af37cba5cb78014e0714c654db2501151f605e Gerrit-Change-Number: 29078 Gerrit-PatchSet: 1 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be>

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by David Hendricks (Code Review) 10 Oct '18

10 Oct '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Patch Set 25: (4 comments) https://review.coreboot.org/#/c/23203/22/flashrom.8.tmpl File flashrom.8.tmpl: https://review.coreboot.org/#/c/23203/22/flashrom.8.tmpl@52 PS22, Line 52: .SH DESCRIPTION > Hmmm, new gerrit UI refused to save after editing above comment... […] Thanks for clarifying, done. As you say it prints as follows now: flashrom [-h|-R|-L|-z|-p <programmername>[:<parameters>] [-E|-r <file>|-w <file>|-v <file>] [-c <chipname>] [(-l <file>|--ifd| --fmap|--fmap-file <file>) [-i <image>]] [-n] [-N] [-f]] [-V[V[V]]] [-o <logfile>] https://review.coreboot.org/#/c/23203/22/fmap.c File fmap.c: https://review.coreboot.org/#/c/23203/22/fmap.c@203 PS22, Line 203: } > Now the malloc error path is missing the finalize_flash_access()... d'oh... fixed. https://review.coreboot.org/#/c/23203/24/fmap.c File fmap.c: https://review.coreboot.org/#/c/23203/24/fmap.c@281 PS24, Line 281: * should be a valid, usable fmap. */ > Should we set `ret = 2` here too? Yes, that makes sense. https://review.coreboot.org/#/c/23203/24/fmap.c@285 PS24, Line 285: : *fmap_out = fmap; : ret = 0; : _free_ret: : if (ret) : free(fmap); : finalize_flash_access(flashctx); > Now that the code is more straight forward to read. We don't need […] Yep, done. -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 25 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Thu, 11 Oct 2018 04:48:34 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by David Hendricks (Code Review) 10 Oct '18

10 Oct '18

David Hendricks has uploaded a new patch set (#25) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Add support to get layout from fmap (e.g. coreboot rom) Flashmap, or simply fmap, is a binary data format for describing region offsets, sizes, and certain attributes and is widely used by coreboot. This patch adds support for the fmap data format version 1.1 and adds --fmap and --fmap-file arguments. Using --fmap will make flashrom to search the ROM content for fmap data. Using --fmap-file will make flashrom search a supplied file for fmap data. An example of how to update the COREBOOT region of a ROM: flashrom -p programmer --fmap -w coreboot.rom -i COREBOOT flashrom -p programmer --fmap-file coreboot.rom -w coreboot.rom -i COREBOOT The fmap functions are mostly copied from cbfstool. Currently it is made mutually exclusive with other layout options until we are more clever about this input. Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M Makefile M cli_classic.c M flashrom.8.tmpl A fmap.c A fmap.h M libflashrom.c M libflashrom.h 7 files changed, 648 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/23203/25 -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 25 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com>

1 0

Change in flashrom[master]: ft2232_spi: add an ability to use GPIO for chip selection
by Nico Huber (Code Review) 08 Oct '18

08 Oct '18

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/28911 ) Change subject: ft2232_spi: add an ability to use GPIO for chip selection ...................................................................... ft2232_spi: add an ability to use GPIO for chip selection Change-Id: I6db05619e0d69ad18549c8556ef69225337b1532 Signed-off-by: Sergey Alirzaev <zl29ah(a)gmail.com> Reviewed-on: https://review.coreboot.org/28911 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M flashrom.8.tmpl M ft2232_spi.c 2 files changed, 26 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index 70af395..f882dc6 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -722,9 +722,9 @@ Multi-Protocol Adapter (TUMPA), TUMPA Lite, GOEPEL PicoTAP and Google Servo v1/v2. .sp An optional parameter specifies the controller -type and channel/interface/port it should support. For that you have to use the +type, channel/interface/port and GPIO-based chip select it should support. For that you have to use the .sp -.B " flashrom \-p ft2232_spi:type=model,port=interface" +.B " flashrom \-p ft2232_spi:type=model,port=interface,csgpiol=gpio" .sp syntax where .B model @@ -733,14 +733,17 @@ arm-usb-tiny ", " arm-usb-tiny-h ", " arm-usb-ocd ", " arm-usb-ocd-h \ ", " tumpa ", " tumpalite ", " picotap ", " google-servo ", " google-servo-v2 \ " or " google-servo-v2-legacy -and .B interface can be -.BR A ", " B ", " C ", or " D . +.BR A ", " B ", " C ", or " D +and +.B csgpiol +can be a number between 0 and 3, denoting GPIOL0-GPIOL3 correspondingly. The default model is .B 4232H -and the default interface is -.BR A . +the default interface is +.BR A +and GPIO is not used by default. .sp If there is more than one ft2232_spi-compatible device connected, you can select which one should be used by specifying its serial number with the diff --git a/ft2232_spi.c b/ft2232_spi.c index 819744c..95584aa 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -327,6 +327,23 @@ } free(arg); + arg = extract_programmer_param("csgpiol"); + if (arg) { + char *endptr; + unsigned int temp = strtoul(arg, &endptr, 10); + if (*endptr || endptr == arg || temp > 3) { + msg_perr("Error: Invalid GPIOL specified: \"%s\".\n" + "Valid values are between 0 and 3.\n", arg); + free(arg); + return -2; + } else { + unsigned int pin = temp + 4; + cs_bits |= 1 << pin; + pindir |= 1 << pin; + } + } + free(arg); + msg_pdbg("Using device type %s %s ", get_ft2232_vendorname(ft2232_vid, ft2232_type), get_ft2232_devicename(ft2232_vid, ft2232_type)); -- To view, visit https://review.coreboot.org/28911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6db05619e0d69ad18549c8556ef69225337b1532 Gerrit-Change-Number: 28911 Gerrit-PatchSet: 7 Gerrit-Owner: Sergey Alirzaev <zl29ah(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergey Alirzaev <zl29ah(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2018