Re: [SeaBIOS] EHCI boot problems
Kevin, Your second patch worked for me. +2. Thanks, Dave On Tue, Sep 9, 2014 at 4:12 PM, Kevin O'Connor wrote: > On Tue, Sep 09, 2014 at 04:08:25PM -0600, Dave Frodin wrote: > > Kevin, > > With your patch I'm not seeing that timeout message get displayed. > > I also forgot to mention that the USB thumbdrive will always get > identified > > by SeaBIOS and will be listed as a boot device but will hang as soon as > it > > is selected in the F12 boot menu. The activity LED on the thumbdrive only > > flashes when Seabios is identifying it. > > I was able to reproduce the problem on my c720. The patch below seems > to fix it for me (but I'm still running tests). I'm not sure why this > problem didn't show up on my other test machine (e350m1). > > -Kevin > > > --- a/src/hw/usb.c > +++ b/src/hw/usb.c > @@ -195,7 +195,7 @@ usb_xfer_time(struct usb_pipe *pipe, int datalen) > // set_address commands where we don't want to stall the boot if > // the device doesn't actually exist. Add 100ms to account for > // any controller delays. > -if (!pipe->devaddr) > +if (!GET_LOWFLAT(pipe->devaddr)) > return USB_TIME_STATUS + 100; > return USB_TIME_COMMAND + 100; > } > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] EHCI boot problems
Kevin, With your patch I'm not seeing that timeout message get displayed. I also forgot to mention that the USB thumbdrive will always get identified by SeaBIOS and will be listed as a boot device but will hang as soon as it is selected in the F12 boot menu. The activity LED on the thumbdrive only flashes when Seabios is identifying it. Thanks, Dave On Tue, Sep 9, 2014 at 2:46 PM, Kevin O'Connor wrote: > On Tue, Sep 09, 2014 at 01:15:04PM -0600, Dave Frodin wrote: > > I'm seeing a problem booting from USB thumbdrives with commit ab9d771ce > > ehci: Update usb command timeouts to use usb_xfer_time() > > > > I'm not quite sure what the problem is other than it not liking the new > > timeouts. > > I couldn't see any problems with the ehci_control() calls to > ehci_wait_td(). > > But the ehci_send_bulk() only allows the system to boot if I change > > int ret = ehci_wait_td(pipe, td, end); > > to > > int ret = ehci_wait_td(pipe, td, timer_calc(5000)); > > > > One potential problem is that the "end" value is calculated once and > reused > > multiple times in the functions. Prior to the commit the timeout value > was > > passed > > to the ehci_wait_td() function. Now the final "end" time is passed. So it > > looks like > > once the end timeout is reached in one of the loops that calls > > ehci_wait_td() the > > timer will always be expired for any other calls from that function. > > > > One solution might be to get rid of the "end" variable and change the > calls > > from > > ret = ehci_wait_td(pipe, td, end); > > to > > ret = ehci_wait_td(pipe, td, timer_calc(usb_xfer_time(p, datasize))); > > > > Making the above change didn't have any impact on my booting problem > until > > I forced the ehci_send_bulk() timeouts to 5000. > > Hi Dave, > > Stefan Burger reported a similar problem with that commit. However, > I'm puzzled what would be causing such a regression - five seconds > should be more then enough time to complete a single usb transaction. > > Do you have serial debugging (or similar) available on the device you > are working with? If you could provide the output from the debugging > patch below it may help diagnose the issue. > > I also got my Acer c720 back, and will run some tests on it. > > -Kevin > > > --- a/src/hw/usb-ehci.c > +++ b/src/hw/usb-ehci.c > @@ -629,6 +629,7 @@ ehci_send_bulk(struct usb_pipe *p, int dir, void > *data, int datasize) > , &pipe->qh, dir, data, datasize); > > // Allocate 4 tds on stack (with required alignment) > +u32 starttime = timer_calc(0); > u32 end = timer_calc(usb_xfer_time(p, datasize)); > u8 tdsbuf[sizeof(struct ehci_qtd) * STACKQTDS + EHCI_QTD_ALIGN - 1]; > struct ehci_qtd *tds = (void*)ALIGN((u32)tdsbuf, EHCI_QTD_ALIGN); > @@ -642,7 +643,7 @@ ehci_send_bulk(struct usb_pipe *p, int dir, void > *data, int datasize) > struct ehci_qtd *td = &tds[tdpos++ % STACKQTDS]; > int ret = ehci_wait_td(pipe, td, end); > if (ret) > -return -1; > +goto fail; > > struct ehci_qtd *nexttd_fl = MAKE_FLATPTR(GET_SEG(SS) > , &tds[tdpos % > STACKQTDS]); > @@ -662,10 +663,15 @@ ehci_send_bulk(struct usb_pipe *p, int dir, void > *data, int datasize) > struct ehci_qtd *td = &tds[tdpos++ % STACKQTDS]; > int ret = ehci_wait_td(pipe, td, end); > if (ret) > -return -1; > +goto fail; > } > > return 0; > +fail: > +dprintf(1, "timeout: start=%x end=%x cur=%x time=%d devaddr=%x > datasize=%d\n" > +, starttime, end, timer_calc(0) > +, usb_xfer_time(p, datasize), p->devaddr, datasize); > +return -1; > } > > int > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] EHCI boot problems
I'm seeing a problem booting from USB thumbdrives with commit ab9d771ce ehci: Update usb command timeouts to use usb_xfer_time() I'm not quite sure what the problem is other than it not liking the new timeouts. I couldn't see any problems with the ehci_control() calls to ehci_wait_td(). But the ehci_send_bulk() only allows the system to boot if I change int ret = ehci_wait_td(pipe, td, end); to int ret = ehci_wait_td(pipe, td, timer_calc(5000)); One potential problem is that the "end" value is calculated once and reused multiple times in the functions. Prior to the commit the timeout value was passed to the ehci_wait_td() function. Now the final "end" time is passed. So it looks like once the end timeout is reached in one of the loops that calls ehci_wait_td() the timer will always be expired for any other calls from that function. One solution might be to get rid of the "end" variable and change the calls from ret = ehci_wait_td(pipe, td, end); to ret = ehci_wait_td(pipe, td, timer_calc(usb_xfer_time(p, datasize))); Making the above change didn't have any impact on my booting problem until I forced the ehci_send_bulk() timeouts to 5000. Thanks in advance for any advice anyone can offer, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Add SD card support
Kevin, Marc suggested that I reword what are intent was/is. We view the SD patch as a starting point and that we would like to work with the community on getting it into seabios. I think that we could use help from yourself and David Woodhouse, who has expressed an interest in it, to improve and further develop the code. Thanks, Dave Frodin On Wed, Jun 11, 2014 at 1:37 PM, Dave Frodin wrote: > Kevin, > Our intent at this time was to just get it out for others to see/use. > I'll try attaching the files. > > Thanks, > Dave > > > > On Tue, Jun 10, 2014 at 6:19 PM, Kevin O'Connor > wrote: > >> On Thu, Jun 05, 2014 at 10:36:07AM -0600, Dave Frodin wrote: >> > Paul, >> > >> > I'm resending the patch, supposedly as plain text. >> > Let me know if you still see a problem. >> > >> > Thanks, >> > Dave >> >> Thanks for posting this Dave. >> >> Is the intention to publish this, or to get this into SeaBIOS master? >> I think the patch will need a bit of work before it could be committed >> to upstream. >> >> [...] >> > +//@NOTE: This step appears to be unnecessary for booting from >> > the sd card as the pchs info does not get used... >> >> BTW, the patch still got corrupted. You might try gzipping it and >> then attaching the gzipped patch. >> >> -Kevin >> > > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Add SD card support
Kevin, Our intent at this time was to just get it out for others to see/use. I'll try attaching the files. Thanks, Dave On Tue, Jun 10, 2014 at 6:19 PM, Kevin O'Connor wrote: > On Thu, Jun 05, 2014 at 10:36:07AM -0600, Dave Frodin wrote: > > Paul, > > > > I'm resending the patch, supposedly as plain text. > > Let me know if you still see a problem. > > > > Thanks, > > Dave > > Thanks for posting this Dave. > > Is the intention to publish this, or to get this into SeaBIOS master? > I think the patch will need a bit of work before it could be committed > to upstream. > > [...] > > +//@NOTE: This step appears to be unnecessary for booting from > > the sd card as the pchs info does not get used... > > BTW, the patch still got corrupted. You might try gzipping it and > then attaching the gzipped patch. > > -Kevin > 0001-Add-SD-card-support.patch.tar.gz Description: GNU Zip compressed data ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] MP Table corruption
Kevin, Comments are inline. Thanks, Dave - Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "seabios" > Sent: Tuesday, October 8, 2013 7:26:25 PM > Subject: Re: [SeaBIOS] MP Table corruption > > On Tue, Oct 08, 2013 at 05:27:34PM -0500, Dave Frodin wrote: > > We have a customer that runs a RTOS that uses the MP Tables (rather > > than ACPI). They are having an issue with the RTOS not being able > > to determine certain system interrupt settings from the MP > > tables. The sent me a MPDiag utility (this may be their own MPDiag > > utility, I believe I've seen others) that exposes the problem. Using > > it I was able to determine that the mptable that coreboot generates > > is getting corrupted somehow by seabios. When I bisected seabios I > > found that there are several commits that cause the mptable > > corruption. The first two problem commits are ... > > 5DBF1732 > > ECA5A947 > > This is on coreboot, right? Yes this is for a coreboot build. > I don't see a commit ECA5A947 in seabios, Oooops. I rebased and deleted 5DBF1732 after I bisected down to it. The 2nd commit that breaks the MPtable is actually A2A86E29. Sorry for the confusion. > and I don't see how 5DBF1732 could impact mptable. I guess it's > possible that 5DBF1732 could impact the interrupts as we no longer run > a sipi on all the processors, but I don't think SeaBIOS should have to > issue a sipi - if that is doing something that impacts the OS then > coreboot should do it as part of initializing the processors. > > -Kevin > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] MP Table corruption
We have a customer that runs a RTOS that uses the MP Tables (rather than ACPI). They are having an issue with the RTOS not being able to determine certain system interrupt settings from the MP tables. The sent me a MPDiag utility (this may be their own MPDiag utility, I believe I've seen others) that exposes the problem. Using it I was able to determine that the mptable that coreboot generates is getting corrupted somehow by seabios. When I bisected seabios I found that there are several commits that cause the mptable corruption. The first two problem commits are ... 5DBF1732 ECA5A947 I reverted both of those commits and still had issues, so for the short term solution I just used a seabios branch based off of 4EDDA08 (which is the commit that precedes 5DBF1732) to allow the customers MPDiag to pass. Has anybody else seen an issue like this? Anybody have any suggestions on how to approach fixing this? Thanks, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Commit b98a4b1 prevents USB drives from being detected
Kevin, Your patch does fix the problem with USB thumbdrives. I tested on a Gizmo (Family14/SB800) board. Thanks, Dave - Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "seabios" > Sent: Sunday, July 14, 2013 11:59:11 AM > Subject: Re: [SeaBIOS] Commit b98a4b1 prevents USB drives from being detected > > On Fri, Jul 12, 2013 at 03:22:43PM -0500, Dave Frodin wrote: > > > I'm using the current seabios (master or stable) as a payload for > > coreboot. If I build the current seabios I've found that commit > > b98a4b1 "Convert PCIDevices list to use standard list manipultion > > code." prevents the system from finding USB thumbdrives. The USB > > keyboard and mouse still work. > > Thanks. The PCI list changes broke the ordering of the PCIDevices > list. Can you confirm the patch below fixes it for you? > > -Kevin > > > commit 2a9aeabdfb34374ecac25e7a8d21c9e368618cd4 > Author: Kevin O'Connor > Date: Sun Jul 14 13:55:52 2013 -0400 > > Fix USB EHCI detection that was broken in hlist conversion of PCIDevices. > > Make sure the PCI device list is ordered in bus order. > > Don't iterate past the end of the list when detecting EHCI devices. > > Signed-off-by: Kevin O'Connor > > diff --git a/src/pci.c b/src/pci.c > index 6163a29..dc62c5c 100644 > --- a/src/pci.c > +++ b/src/pci.c > @@ -122,6 +122,7 @@ pci_probe_devices(void) > } > memset(dev, 0, sizeof(*dev)); > hlist_add(&dev->node, pprev); > +pprev = &dev->node.next; > count++; > > // Find parent device. > diff --git a/src/usb.c b/src/usb.c > index ecccd75..42541ff 100644 > --- a/src/usb.c > +++ b/src/usb.c > @@ -444,7 +444,7 @@ usb_setup(void) > } > if (ehcipci->class == PCI_CLASS_SERIAL_USB) > found++; > -ehcipci = container_of( > +ehcipci = container_of_or_null( > ehcipci->node.next, struct pci_device, node); > if (!ehcipci || (pci_bdf_to_busdev(ehcipci->bdf) > != pci_bdf_to_busdev(pci->bdf))) > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] Commit b98a4b1 prevents USB drives from being detected
I'm using the current seabios (master or stable) as a payload for coreboot. If I build the current seabios I've found that commit b98a4b1 "Convert PCIDevices list to use standard list manipultion code." prevents the system from finding USB thumbdrives. The USB keyboard and mouse still work. This problem occurs for multiple mainboards (persimmon/parmer/gizmo). Let me know if there's something else I can try for anybody. Thanks, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
Kevin, I talked with Marc Jones about this and he sees value in the method I've proposed. He also predicts that this will be an issue in the future on Intel motherboards. The one major difference is that if you were building coreboot for a Family15 mainboard you would only need to add one file (vendev_map) rather than 20 alias mapping files to CBFS. I agree that the parsing code is more than clunky (what I wouldn't give for a sscanf()), perhaps my patch could use some changes rather than throwing it out. I'll re-attach my patch in case anybody wants to review it. Thanks again, Dave - Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "seabios" > Sent: Tuesday, July 2, 2013 10:29:39 PM > Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option > ROMs to one > > On Tue, Jul 02, 2013 at 07:40:17AM -0500, Dave Frodin wrote: > > Kevin, > > > > Did you ever get a chance to look over this patch? > > I don't see the gain in introducing a config file parser for this > task. What was wrong with the approach of creating "alias" files? > > -Kevin > From 907686ae6a41a0cb75d0ff2a565586b6f4e25615 Mon Sep 17 00:00:00 2001 From: Dave Frodin Date: Thu, 23 May 2013 16:07:17 -0600 Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one This feature was added to allow mapping multiple different PCI vendor/device IDs to a single ID. It allows a coreboot/seabios ROM image to be used on a mainboard whose graphics ID will vary depending on which cpu is installed. The intent is to have the coreboot mainboard define its VGA_BIOS_ID as the ID that is present in the actual VGA BIOS. The PCI ID of all possible graphics IDs would then be mapped to that ID. It may have use for other PCI devices where a single option ROM can be used with PCI devices with varying IDs. Signed-off-by: Dave Frodin --- src/optionroms.c | 87 +++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/src/optionroms.c b/src/optionroms.c index ac92613..3b86244 100644 --- a/src/optionroms.c +++ b/src/optionroms.c @@ -177,13 +177,98 @@ deploy_romfile(struct romfile_s *file) return rom; } +/* Read up to 8 ascii chars plus one non-convertible character from a + * string and convert to binary. + */ +static u32 +text_to_binary( char **ptr ) +{ +u32 data = 0; +u8 loop = 0; +if (*ptr == NULL) +return data; +for ( ; ; (*ptr)++ ) { +if ((**ptr >= '0') && ( **ptr <= '9')) +data = (data * 0x10) + (**ptr - '0'); +else if ((**ptr >= 'a') && (**ptr <= 'f')) +data = (data * 0x10) + (**ptr - 'a' + 0x0A); +else if ((**ptr >= 'A') && (**ptr <= 'F')) +data = (data * 0x10) + (**ptr - 'A' + 0x0A); +else +break; +if (++loop >= 9) +break; +} +(*ptr)++; /* point to the char after the non-convertible char */ +return data; +} + +/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD + * VGA BIOS will quite often be used on hardware that reports different + * PCI graphics IDs. This allows a mainboard to have a single definition + * (which would match the ID in the option ROM) yet would support multiple + * CPU IDs. + * Example vendev-map format for AMD Family14 (ID=1002) vgabios. + *10029803-10029802 + *10029804-10029802 + *10029805-10029802 + *10029806-10029802 + *10029807-10029802 + *10029808-10029802 + *10029809-10029802 + */ +#define MAP_FILE_STATUS_UNINITED 0 +#define MAP_FILE_STATUS_GOOD 1 +#define MAP_FILE_STATUS_NOEXIST 2 +#define MAP_FILE_STATUS_USED 3 +#define MAPPER_BYTES_PER_LINE18 /* U32 src, '-', U32 dst, 0x0A */ + +static u32 +map_oprom_vendev(u32 vendev) +{ +static char *filedata = NULL; +static int filesize = 0, map_status = MAP_FILE_STATUS_UNINITED; +u32 vendev_src = 0, vendev_dst = 0; +char *tmp_ptr; +int i; + +if ((map_status == MAP_FILE_STATUS_NOEXIST) || +(map_status == MAP_FILE_STATUS_USED)) +return vendev; + +if (map_status == MAP_FILE_STATUS_UNINITED) { +filedata = romfile_loadfile("vendev-map", &filesize); +if (!filedata) { +map_status = MAP_FILE_STATUS_NOEXIST; +return vendev; +} +else +map_status = MAP_FILE_STATUS_GOOD; +} + +tmp_ptr = filedata; +for (i=0; i < filesize/MAPPER_BYTES_PER_LINE; i++) { +vendev_src = text_to_binary( &tmp_ptr ); +vendev_dst = text_to_binary( &tmp_ptr ); +dprintf(5, "map %d: src=%x dst=%x\n", i, vendev_src, vendev_dst); +if ( vendev
Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
Kevin, Did you ever get a chance to look over this patch? Thanks, Dave - Original Message - > From: "Dave Frodin" > To: "Kevin O'Connor" > Cc: "seabios" > Sent: Wednesday, June 5, 2013 8:45:11 AM > Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option > ROMs to one > > Kevin, > > Attached is the new option rom mapping patch. > I was hunting around for an existing SeaBIOS ftn that would convert > ascii strings to a binary value (i.e. sscanf) but couldn't find one. > If there's already one in the code I can replace the text_to_binary() > with it. > > I tested this on a AMD family14 platform. The vendev-map file I added > to CBFS looks like this... > > 10029803-10029802 > 10029804-10029802 > 10029805-10029802 > 10029806-10029802 > 10029807-10029802 > 10029808-10029802 > 10029809-10029802 > > Each line ends with a NEWLINE character (0x0A). > > On my test platform the actual graphics ID of the part is 10029804. > The vgabios gets added to CBFS with the name "pci1002,9802.rom". > > Thanks, > Dave > ___ > SeaBIOS mailing list > SeaBIOS@seabios.org > http://www.seabios.org/mailman/listinfo/seabios > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
Kevin, Attached is the new option rom mapping patch. I was hunting around for an existing SeaBIOS ftn that would convert ascii strings to a binary value (i.e. sscanf) but couldn't find one. If there's already one in the code I can replace the text_to_binary() with it. I tested this on a AMD family14 platform. The vendev-map file I added to CBFS looks like this... 10029803-10029802 10029804-10029802 10029805-10029802 10029806-10029802 10029807-10029802 10029808-10029802 10029809-10029802 Each line ends with a NEWLINE character (0x0A). On my test platform the actual graphics ID of the part is 10029804. The vgabios gets added to CBFS with the name "pci1002,9802.rom". Thanks, DaveFrom 907686ae6a41a0cb75d0ff2a565586b6f4e25615 Mon Sep 17 00:00:00 2001 From: Dave Frodin Date: Thu, 23 May 2013 16:07:17 -0600 Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one This feature was added to allow mapping multiple different PCI vendor/device IDs to a single ID. It allows a coreboot/seabios ROM image to be used on a mainboard whose graphics ID will vary depending on which cpu is installed. The intent is to have the coreboot mainboard define its VGA_BIOS_ID as the ID that is present in the actual VGA BIOS. The PCI ID of all possible graphics IDs would then be mapped to that ID. It may have use for other PCI devices where a single option ROM can be used with PCI devices with varying IDs. Signed-off-by: Dave Frodin --- src/optionroms.c | 87 +++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/src/optionroms.c b/src/optionroms.c index ac92613..3b86244 100644 --- a/src/optionroms.c +++ b/src/optionroms.c @@ -177,13 +177,98 @@ deploy_romfile(struct romfile_s *file) return rom; } +/* Read up to 8 ascii chars plus one non-convertible character from a + * string and convert to binary. + */ +static u32 +text_to_binary( char **ptr ) +{ +u32 data = 0; +u8 loop = 0; +if (*ptr == NULL) +return data; +for ( ; ; (*ptr)++ ) { +if ((**ptr >= '0') && ( **ptr <= '9')) +data = (data * 0x10) + (**ptr - '0'); +else if ((**ptr >= 'a') && (**ptr <= 'f')) +data = (data * 0x10) + (**ptr - 'a' + 0x0A); +else if ((**ptr >= 'A') && (**ptr <= 'F')) +data = (data * 0x10) + (**ptr - 'A' + 0x0A); +else +break; +if (++loop >= 9) +break; +} +(*ptr)++; /* point to the char after the non-convertible char */ +return data; +} + +/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD + * VGA BIOS will quite often be used on hardware that reports different + * PCI graphics IDs. This allows a mainboard to have a single definition + * (which would match the ID in the option ROM) yet would support multiple + * CPU IDs. + * Example vendev-map format for AMD Family14 (ID=1002) vgabios. + *10029803-10029802 + *10029804-10029802 + *10029805-10029802 + *10029806-10029802 + *10029807-10029802 + *10029808-10029802 + *10029809-10029802 + */ +#define MAP_FILE_STATUS_UNINITED 0 +#define MAP_FILE_STATUS_GOOD 1 +#define MAP_FILE_STATUS_NOEXIST 2 +#define MAP_FILE_STATUS_USED 3 +#define MAPPER_BYTES_PER_LINE18 /* U32 src, '-', U32 dst, 0x0A */ + +static u32 +map_oprom_vendev(u32 vendev) +{ +static char *filedata = NULL; +static int filesize = 0, map_status = MAP_FILE_STATUS_UNINITED; +u32 vendev_src = 0, vendev_dst = 0; +char *tmp_ptr; +int i; + +if ((map_status == MAP_FILE_STATUS_NOEXIST) || +(map_status == MAP_FILE_STATUS_USED)) +return vendev; + +if (map_status == MAP_FILE_STATUS_UNINITED) { +filedata = romfile_loadfile("vendev-map", &filesize); +if (!filedata) { +map_status = MAP_FILE_STATUS_NOEXIST; +return vendev; +} +else +map_status = MAP_FILE_STATUS_GOOD; +} + +tmp_ptr = filedata; +for (i=0; i < filesize/MAPPER_BYTES_PER_LINE; i++) { +vendev_src = text_to_binary( &tmp_ptr ); +vendev_dst = text_to_binary( &tmp_ptr ); +dprintf(5, "map %d: src=%x dst=%x\n", i, vendev_src, vendev_dst); +if ( vendev_src == vendev) { +dprintf(1, "Mapping PCI device %8x to %8x\n",vendev, vendev_dst); +map_status = MAP_FILE_STATUS_USED; +free(filedata); +return vendev_dst; +} +} +return vendev; +} + // Check if an option rom is at a hardcoded location or in CBFS. static struct rom_header * lookup_hardcode(struct pci_device *pci) { char fname[17]; +u32 vendev = map_oprom_vendev((pci->vendor << 16) | pci->device); snprintf(fname, size
Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
- Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "seabios" > Sent: Friday, May 31, 2013 9:18:21 PM > Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option > ROMs to one > > On Fri, May 31, 2013 at 08:32:16AM -0500, Dave Frodin wrote: > > I see now, that's pretty slick. The problem I see with that approach is > > that coreboot would need to add all of these files for a family14 > > mainboard. > [...] > > And all of these files for a family15 mainboard > [...] > > > > Since at build time we only know what cpu family the rom is being > > built for and not (necessarily) what the exact graphics ID is. > > > > The method I proposed would have one family specific "vendev-map.bin" file. > > Yes, but it would be quite difficult to inspect vendev-map.bin. I > think an ascii interface is preferable. > > One could go a level up and make a "symbolic link" type in cbfs (this > is effectively what my patch did). The advantage would be that > cbfstool could display it in a nicer format. > > -Kevin I agree with that, and marc made the same suggestion. I'll put together a new patch next week. Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
- Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "seabios" > Sent: Thursday, May 30, 2013 7:45:13 PM > Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option > ROMs to one > > On Thu, May 30, 2013 at 07:49:36AM -0500, Dave Frodin wrote: > > > From: "Kevin O'Connor" > > > --- a/src/optionroms.c > > > +++ b/src/optionroms.c > > > @@ -178,10 +178,19 @@ deploy_romfile(struct romfile_s *file) > > > static struct rom_header * > > > lookup_hardcode(struct pci_device *pci) > > > { > > > -char fname[17]; > > > -snprintf(fname, sizeof(fname), "pci%04x,%04x.rom" > > > +struct romfile_s *file; > > > +char fname[19]; > > > +snprintf(fname, sizeof(fname), "alias%04x,%04x.rom" > > > , pci->vendor, pci->device); > > > -struct romfile_s *file = romfile_find(fname); > > > +char *alias = romfile_loadfile(fname, NULL); > > > +if (alias) { > > > +file = romfile_find(alias); > > > +free(alias); > > > +} else { > > > +snprintf(fname, sizeof(fname), "pci%04x,%04x.rom" > > > + , pci->vendor, pci->device); > > > +file = romfile_find(fname); > > > +} > > > if (file) > > > return deploy_romfile(file); > > > return NULL; > > > > > > > In your sample code above, I don't see where any pci ID translation > > (mapping) occurs. > > As an example we could have a family14 mainboard with a coreboot > > .config that generates a vga option rom with the name > > "pci1002,9802.rom". The actual mainboard could have a graphics chip > > with the id of 1002,9804 (which is 1 of 8 possible IDs). So there > > needs to be some sort of mapping function (in SeaBIOS) that maps the > > ID of 1002,9804 to 1002,9802. > > With the above patch (assuming it works) one should be able to create > a CBFS file named "alias1002,9804.rom" that contains the text > "pci1002,9802.rom". > > -Kevin I see now, that's pretty slick. The problem I see with that approach is that coreboot would need to add all of these files for a family14 mainboard. alias1002,9803.rom alias1002,9804.rom alias1002,9805.rom alias1002,9806.rom alias1002,9807.rom alias1002,9808.rom alias1002,9809.rom And all of these files for a family15 mainboard alias1002,9901.rom alias1002,9903.rom alias1002,9904.rom alias1002,9906.rom alias1002,9907.rom alias1002,9908.rom alias1002,990a.rom alias1002,9910.rom alias1002,9913.rom alias1002,9917.rom alias1002,9918.rom alias1002,9919.rom alias1002,9990.rom alias1002,9991.rom alias1002,9992.rom alias1002,9993.rom alias1002,9994.rom alias1002,99a0.rom alias1002,99a2.rom alias1002,99a4.rom Since at build time we only know what cpu family the rom is being built for and not (necessarily) what the exact graphics ID is. The method I proposed would have one family specific "vendev-map.bin" file. Someone who's building a rom for a mainboard with a single known graphics ID wouldn't need to add any files with either method. Thanks, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
- Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "seabios" > Sent: Saturday, May 25, 2013 10:47:42 AM > Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option > ROMs to one > > On Fri, May 24, 2013 at 09:06:54AM -0500, Dave Frodin wrote: > > Kevin, > > > > Thanks. > > The new patch that I've attached no longer alters the pci device struct. > > I also no longer repeatedly read the file in from CBFS, but keep the > > pointer to it around for the next time the function is called. > > > > As far as the use case is concerned. We (or a client) will often have > > several of > > the same mainboard (e.g. persimmon), but the graphics ID will vary > > depending on > > which version of the cpu is installed. Previously, we would have to figure > > out > > what the PCI ID of the graphics devices was, adjust the ID in coreboot's > > config, > > build/flash the new rom image. This seabios change along with a change to > > coreboot to set the ID and stuff the correct vendev mapping file into CBFS > > will allow a single coreboot/seabios image to be used on any particular > > mainboard independent of what cpu was installed. This change won't be > > useful > > to someone who is using a single board who's graphics ID would never > > change. > > For that case they wouldn't need to do anything. If the vendev mapping file > > doesn't get added they would see no difference. > > Thanks. > > Why not just place the rom in "vgaroms/" directory where it will > always be run? That is an option but I was hoping to reduce the number of configuration differences between having coreboot vs seabios load/run the vga option rom. Currently, coreboot has vendor/device ID mapping functions for the family14, family15tn northbridges. > Also, I think we could avoid the binary structure in CBFS. Something > like the below (totally untested). > > -Kevin > > > --- a/src/optionroms.c > +++ b/src/optionroms.c > @@ -178,10 +178,19 @@ deploy_romfile(struct romfile_s *file) > static struct rom_header * > lookup_hardcode(struct pci_device *pci) > { > -char fname[17]; > -snprintf(fname, sizeof(fname), "pci%04x,%04x.rom" > +struct romfile_s *file; > +char fname[19]; > +snprintf(fname, sizeof(fname), "alias%04x,%04x.rom" > , pci->vendor, pci->device); > -struct romfile_s *file = romfile_find(fname); > +char *alias = romfile_loadfile(fname, NULL); > +if (alias) { > +file = romfile_find(alias); > +free(alias); > +} else { > +snprintf(fname, sizeof(fname), "pci%04x,%04x.rom" > + , pci->vendor, pci->device); > +file = romfile_find(fname); > +} > if (file) > return deploy_romfile(file); > return NULL; > In your sample code above, I don't see where any pci ID translation (mapping) occurs. As an example we could have a family14 mainboard with a coreboot .config that generates a vga option rom with the name "pci1002,9802.rom". The actual mainboard could have a graphics chip with the id of 1002,9804 (which is 1 of 8 possible IDs). So there needs to be some sort of mapping function (in SeaBIOS) that maps the ID of 1002,9804 to 1002,9802. Thanks again, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
Kevin, Thanks. The new patch that I've attached no longer alters the pci device struct. I also no longer repeatedly read the file in from CBFS, but keep the pointer to it around for the next time the function is called. As far as the use case is concerned. We (or a client) will often have several of the same mainboard (e.g. persimmon), but the graphics ID will vary depending on which version of the cpu is installed. Previously, we would have to figure out what the PCI ID of the graphics devices was, adjust the ID in coreboot's config, build/flash the new rom image. This seabios change along with a change to coreboot to set the ID and stuff the correct vendev mapping file into CBFS will allow a single coreboot/seabios image to be used on any particular mainboard independent of what cpu was installed. This change won't be useful to someone who is using a single board who's graphics ID would never change. For that case they wouldn't need to do anything. If the vendev mapping file doesn't get added they would see no difference. Thanks again, Dave - Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "seabios" > Sent: Thursday, May 23, 2013 9:01:00 PM > Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option > ROMs to one > > On Thu, May 23, 2013 at 05:25:42PM -0500, Dave Frodin wrote: > > Kevin, > > I've attached a patch that I hope is more along the lines > > of where you wanted me to go with this change. In my testing > > of the patch I'm currently just stuffing the vendev-map.bin file > > into my coreboot image. > > Thanks. > > [...] > > +for (i=0; i < filesize/(sizeof (u32)); i+=2) { > > +if ( filedata[i] == ((pci->vendor << 16) | pci->device)) { > > +dprintf(1, "Mapping PCI device 0x%8x to 0x%8x\n", > > +((pci->vendor << 16) | pci->device), filedata[i+1]); > > +pci->vendor = filedata[i+1] >> 16; > > +pci->device = filedata[i+1] & 0x; > > +return; > > I don't understand why struct pci_device is being modified. Maybe you > could further explain the use case. After modifying struct > pci_device, where does the video rom actually come from? > > -Kevin > From f54a6a23e12cb8f76c46248d8effe7b07abfdaf3 Mon Sep 17 00:00:00 2001 From: Dave Frodin Date: Thu, 23 May 2013 16:07:17 -0600 Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one This feature was added to allow mapping multiple different PCI vendor/device IDs to a single ID. It allows a coreboot/seabios ROM image to be used on a mainboard whose graphics ID will vary depending on which cpu is installed. The intent is to have the coreboot mainboard define its VGA_BIOS_ID as the ID that is present in the actual VGA BIOS. The PCI ID of all possible graphics IDs would then be mapped to that ID. It may have use for other PCI devices where a single option ROM can be used with PCI devices with varying IDs. Signed-off-by: Dave Frodin --- src/optionroms.c | 48 +++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/src/optionroms.c b/src/optionroms.c index ac92613..4f37560 100644 --- a/src/optionroms.c +++ b/src/optionroms.c @@ -177,13 +177,59 @@ deploy_romfile(struct romfile_s *file) return rom; } +/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD + * VGA BIOS will quite often be used on hardware that reports different + * PCI graphics IDs. This allows a mainboard to have a single definition + * (which would match the ID in the option ROM) yet would support multiple + * CPU IDs. + * Example vendev-map.bin format (shown as text) of two AMD (ID=1002) vgabios + * getting mapped from 9803/9804 to 9802 + * From To + * 10029803 10029802 + * 10029804 10029802 + */ +#define MAP_FILE_STATUS_UNINITED 0 +#define MAP_FILE_STATUS_GOOD 1 +#define MAP_FILE_STATUS_BAD 2 + +static u32 +map_oprom_vendev(u32 vendev) +{ +static u32 *filedata = NULL; +static int filesize = 0, map_status = MAP_FILE_STATUS_UNINITED; +u32 new_vendev = vendev; +int i; + +// On first call see if the file exists and cache it +if (map_status == MAP_FILE_STATUS_UNINITED) { +filedata = romfile_loadfile("vendev-map.bin", &filesize); +if (!filedata) +map_status = MAP_FILE_STATUS_BAD; +else +map_status = MAP_FILE_STATUS_GOOD; +} + +if (map_status != MAP_FILE_STATUS_GOOD) +return vendev; + +for (i=0; i < filesize/(sizeof (u32)); i+=2) { + if ( filedata[i] == vendev) { +dprintf(1, "Mapping PCI device 0x%8x to 0x%8x\n",vendev, filedata[
Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
Kevin, I've attached a patch that I hope is more along the lines of where you wanted me to go with this change. In my testing of the patch I'm currently just stuffing the vendev-map.bin file into my coreboot image. Thanks, Dave - Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "seabios" > Sent: Wednesday, May 22, 2013 6:45:34 AM > Subject: Re: [SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option > ROMs to one > > On Tue, May 21, 2013 at 02:47:18PM -0500, Dave Frodin wrote: > > All, > > This is a patch that reproduces the vendor ID mapping that is done in > > coreboot in the various AMD northbridge's. The coreboot mapping is only > > useful if coreboot is used to run the vga bios. If seabios is the payload > > then most coreboot configs leave the vga bios init for it. > > Thanks. See my comments below. > > [...] > > --- a/src/optionroms.c > > +++ b/src/optionroms.c > > @@ -154,7 +154,6 @@ getRomPriority(u64 *sources, struct rom_header *rom, > > int instance) > > return bootprio_find_named_rom(file->name, instance); > > } > > Looks like whitespace got corrupted in your email. > > [...] > > +static u32 > > +map_oprom_vendev(u32 vendev) > > +{ > > + u32 new_vendev = vendev; > > + > > + switch (vendev) { > > + case 0x10029803: // Family14 > > + case 0x10029804: > > If we're going to have a mapping, I think it has to be read from > CBFS. I don't think we should hardcode a list into seabios. > > [...] > > static struct rom_header * > > lookup_hardcode(struct pci_device *pci) > > { > > char fname[17]; > > + u32 vendev_mapped; > > + > > + vendev_mapped = map_oprom_vendev((pci->vendor << 16) | pci->device); > > + pci->vendor = vendev_mapped >> 16; > > + pci->device = vendev_mapped & 0x; > > Modifying struct pci_device doesn't look right. > > -Kevin > From 7538a044131c12eda885614ad8a0b1fcdd9b9364 Mon Sep 17 00:00:00 2001 From: Dave Frodin Date: Thu, 23 May 2013 16:07:17 -0600 Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one This feature was added to allow mapping multiple different PCI graphics vendor/device IDs to a single ID. The intent is to have the coreboot mainboard define its VGA_BIOS_ID as the ID that is present in the actual VGA BIOS. The PCI ID of the graphics device would then be mapped to that ID. Signed-off-by: Dave Frodin --- src/optionroms.c | 33 + 1 file changed, 33 insertions(+) diff --git a/src/optionroms.c b/src/optionroms.c index ac92613..6361217 100644 --- a/src/optionroms.c +++ b/src/optionroms.c @@ -177,11 +177,44 @@ deploy_romfile(struct romfile_s *file) return rom; } +/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD + * VGA BIOS will quite often be used on hardware that reports different + * PCI graphics IDs. This allows a mainboard to have a single definition + * (which would match the ID in the VGA BIOS) yet would support multiple + * CPU IDs. + * Example vendev-map.bin format (shown as text) of two AMD (1002) vgabios + * getting mapped from 9803/9804 to 9802 + * From To + * 10029803 10029802 + * 10029804 10029802 + */ +static void +map_oprom_vendev(struct pci_device *pci) +{ +int filesize, i; + +u32 *filedata = romfile_loadfile("vendev-map.bin", &filesize); +if (!filedata) +return; + +for (i=0; i < filesize/(sizeof (u32)); i+=2) { +if ( filedata[i] == ((pci->vendor << 16) | pci->device)) { +dprintf(1, "Mapping PCI device 0x%8x to 0x%8x\n", +((pci->vendor << 16) | pci->device), filedata[i+1]); +pci->vendor = filedata[i+1] >> 16; +pci->device = filedata[i+1] & 0x; +return; +} +} +return; +} + // Check if an option rom is at a hardcoded location or in CBFS. static struct rom_header * lookup_hardcode(struct pci_device *pci) { char fname[17]; +map_oprom_vendev(pci); snprintf(fname, sizeof(fname), "pci%04x,%04x.rom" , pci->vendor, pci->device); struct romfile_s *file = romfile_find(fname); -- 1.7.9.5 ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one
All, This is a patch that reproduces the vendor ID mapping that is done in coreboot in the various AMD northbridge's. The coreboot mapping is only useful if coreboot is used to run the vga bios. If seabios is the payload then most coreboot configs leave the vga bios init for it. >From 5b7f2ba9f43fbc67a81a2449d8bbd3d2f6e530aa Mon Sep 17 00:00:00 2001 From: Dave Frodin Date: Tue, 7 May 2013 13:51:56 -0600 Subject: [PATCH] Seabios: allow mapping of multiple PCI option ROMs to one This feature was added to allow mapping multiple different PCI graphics vendor/device IDs to a single ID. The intent is to have the coreboot mainboard define its VGA_BIOS_ID as the ID that is present in the actual VGA BIOS. The PCI ID of the graphics device would then be mapped to that ID. Change-Id: Id06a1c9730546070146932a4dc8ab8229c4a59b9 Signed-off-by: Dave Frodin --- src/optionroms.c | 69 +- 1 files changed, 68 insertions(+), 1 deletions(-) diff --git a/src/optionroms.c b/src/optionroms.c index 00697b2..bf1e977 100644 --- a/src/optionroms.c +++ b/src/optionroms.c @@ -154,7 +154,6 @@ getRomPriority(u64 *sources, struct rom_header *rom, int instance) return bootprio_find_named_rom(file->name, instance); } - / * Roms in CBFS / @@ -174,11 +173,79 @@ deploy_romfile(struct romfile_s *file) return rom; } +/* Allow mapping of multiple different PCI IDs to a single ID. A single AMD + * VGA BIOS will quite often be used on hardware that reports different + * PCI graphics IDs. This allows a mainboard to have a single definition + * (which would match the ID in the VGA BIOS) yet would support multiple + * CPU IDs. + */ +static u32 +map_oprom_vendev(u32 vendev) +{ + u32 new_vendev = vendev; + + switch (vendev) { + case 0x10029803: // Family14 + case 0x10029804: + case 0x10029805: + case 0x10029806: + case 0x10029807: + case 0x10029808: + case 0x10029809: + new_vendev = 0x10029802; + break; + case 0x10029901: // Family15tn + case 0x10029903: + case 0x10029904: + case 0x10029906: + case 0x10029907: + case 0x10029908: + case 0x1002990A: + case 0x10029910: + case 0x10029913: + case 0x10029917: + case 0x10029918: + case 0x10029919: + case 0x10029990: + case 0x10029991: + case 0x10029992: + case 0x10029993: + case 0x10029994: + case 0x100299A0: + case 0x100299A2: + case 0x100299A4: + new_vendev = 0x10029900; + break; + case 0x10029831: // Family16kb + case 0x10029832: + case 0x10029833: + case 0x10029834: + case 0x10029835: + case 0x10029836: + case 0x10029837: + case 0x10029839: + case 0x1002983D: + new_vendev = 0x10029830; + break; + default: + break; + } + if (vendev != new_vendev) + dprintf(1, "Mapping PCI device %8x to %8x\n",vendev, new_vendev); + return new_vendev; +} + // Check if an option rom is at a hardcoded location or in CBFS. static struct rom_header * lookup_hardcode(struct pci_device *pci) { char fname[17]; + u32 vendev_mapped; + + vendev_mapped = map_oprom_vendev((pci->vendor << 16) | pci->device); + pci->vendor = vendev_mapped >> 16; + pci->device = vendev_mapped & 0x; + snprintf(fname, sizeof(fname), "pci%04x,%04x.rom" , pci->vendor, pci->device); struct romfile_s *file = romfile_find(fname); -- 1.7.9 ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Get rid of a compile time dprintf warning
Thanks for looking into this. I'm using gcc version 4.6.3 > > > > I'm building this in cygwin, and using our (Sage's) toolchain. > > What version of gcc is it? > > If I change the format to %ld I get: > > Compile checking out/post.o > src/post.c: In function ‘reloc_init’: > src/post.c:350:5: warning: format ‘%ld’ expects argument of type > ‘long int’, but argument 4 has type ‘int’ [-Wformat] > > Which is what I'd expect. Maybe there is something odd with your > version of gcc? > > $ gcc --version > gcc (GCC) 4.7.2 20120921 (Red Hat 4.7.2-2) > > and I get similar behavior with gcc v3.4. > > -Kevin > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Get rid of a compile time dprintf warning
Kevin, Here are some of the lines from the output stream... Compile checking out/esp-scsi.o Compile checking out/megasas.o Compile checking out/post.o src/post.c: In function 'reloc_init': src/post.c:350:5: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long int' [-Wformat] Compile checking out/shadow.o Compile checking out/memmap.o Compile checking out/coreboot.o I'm building this in cygwin, and using our (Sage's) toolchain. thanks, Dave - Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "seabios" > Sent: Wednesday, January 9, 2013 4:54:28 PM > Subject: Re: [SeaBIOS] Get rid of a compile time dprintf warning > > On Wed, Jan 09, 2013 at 08:34:18AM -0600, Dave Frodin wrote: > > Here's a patch that's been lingering awhile. > > thanks, > > Thanks. I don't receive a warning for this - what is the exact > warning you receive? I don't see why gcc would convert (datalow_end > - > datalow_start) to a long. > > -Kevin > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Get rid of a compile time dprintf warning
Here's a patch that's been lingering awhile. thanks, Dave - Original Message - > From: "Dave Frodin" > To: "seabios" > Sent: Wednesday, October 17, 2012 9:30:01 AM > Subject: Get rid of a compile time dprintf warning > This patch gets rid of a compile time dprintf warning. > A 'long int' was being passed to a '%d'. > Signed-off-by: Dave Frodin > --- > src/post.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > diff --git a/src/post.c b/src/post.c > index 924b311..b84d19e 100644 > --- a/src/post.c > +++ b/src/post.c > @@ -345,7 +345,7 @@ reloc_init(void) > panic("No space for init relocation.\n"); > // Copy code and update relocs (init absolute, init relative, and > runtime) > - dprintf(1, "Relocating low data from %p to %p (size %d)\n" > + dprintf(1, "Relocating low data from %p to %p (size %ld)\n" > , datalow_start, final_datalow_start, datalow_end - datalow_start); > updateRelocs(code32flat_start, _reloc_datalow_start, > _reloc_datalow_end > , final_datalow_start - datalow_start); > -- > 1.7.9 ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] I2C console16/32 segment problems
I found a solution to my problem. My I2C code relies on using several "static int" variables. In order to get the code to build I had to change them to global and add the VAR16VISIBLE definition. int i2c_inited VAR16VISIBLE = 0; My "static" i2c_inited variable doesn't need global visibility. Is there a better way to do this in SeaBIOS so that they are static local variables rather than global and still satisfy the 16/32 segment stuff? thanks in advance, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] Get rid of a compile time dprintf warning
This patch gets rid of a compile time dprintf warning. A 'long int' was being passed to a '%d'. Signed-off-by: Dave Frodin --- src/post.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/post.c b/src/post.c index 924b311..b84d19e 100644 --- a/src/post.c +++ b/src/post.c @@ -345,7 +345,7 @@ reloc_init(void) panic("No space for init relocation.\n"); // Copy code and update relocs (init absolute, init relative, and runtime) - dprintf(1, "Relocating low data from %p to %p (size %d)\n" + dprintf(1, "Relocating low data from %p to %p (size %ld)\n" , datalow_start, final_datalow_start, datalow_end - datalow_start); updateRelocs(code32flat_start, _reloc_datalow_start, _reloc_datalow_end , final_datalow_start - datalow_start); -- 1.7.9 ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] I2C console16/32 segment problems
All, I've added code to output.c to implement a I2C console interface. Everything was working great on my AMD/Persimmon until I added additional code to determine what south bridge I was on. I'm evidently not correctly handling the 16/32 bit code segments correctly. It's evident when I run the debugger and watch the disassembly. Not only is the disassembly wrong but it isn't even consistent between runs. The new code is below, along with the modified putc_debug(). Any suggestions would be appreciated. Thanks, Dave #if MODE16 #define FIXUP_CODE_SEG_LOCATION VAR16 #elif MODESEGMENT #define FIXUP_CODE_SEG_LOCATION VAR32SEG #else #define FIXUP_CODE_SEG_LOCATION #endif struct amd_i2c_list{ u16 sb_ven; u16 sb_dev; u8 sb_ver; u8 base_access; }; /* send a debug character to the i2c port */ static void debug_i2c(char data) { static int FIXUP_CODE_SEG_LOCATION i2c_amd_iobase = 0; static int FIXUP_CODE_SEG_LOCATION i2c_inited = 0; static const FIXUP_CODE_SEG_LOCATION struct amd_i2c_list amd_smbus_table[] = { { PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_SBX00_SMBUS, 0x39, 2 }, /* ATI/AMD SB700 base in pci space */ { PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_SBX00_SMBUS, 0x42, 1 }, /* ATI/AMD SB800 base in pmreg */ { PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_SB900_SM, 0x14, 1 }, /* ATI/AMD SB900 base in pmreg */ }; volatile u16 rd_vendor, rd_device; volatile u8 rd_ver, base_access; int i; u32 x; if (!CONFIG_DEBUG_I2C) return; /* check to see if we already tried init and it failed */ if (i2c_inited && !i2c_amd_iobase) return; /* check to see if the base address has been initialized */ if (!i2c_inited) { i2c_inited=1; /* check for the AMD/ATI southbridges */ rd_vendor = pci_config_readw(0xA0, 0x00); rd_device = pci_config_readw(0xA0, 0x02); rd_ver = pci_config_readb(0xA0, 0x08); base_access=0; for (i=0;i < sizeof(amd_smbus_table)/sizeof(amd_smbus_table[0]);++i) { if ((amd_smbus_table[i].sb_ven==rd_vendor) && (amd_smbus_table[i].sb_dev==rd_device) && (amd_smbus_table[i].sb_ver==rd_ver)) { base_access = amd_smbus_table[i].base_access; break; } } switch (base_access) { case 1: /* check the PM regs for the SM bus base address */ outb (0x2D, 0xCD6); i2c_amd_iobase = inb(0xCD7)<<8; outb (0x2C, 0xCD6); i2c_amd_iobase |= inb(0xCD7); if ((i2c_amd_iobase!=0) && ((i2c_amd_iobase&0x01)==0x01)) { /* Found a SBx00 base address in PM reg space */ i2c_amd_iobase &= 0xffe0; } else { /* can not find a valid base address */ i2c_amd_iobase = 0; } break; case 2: /* check PCI config space for SM bus base address */ x = pci_config_readl(0xA0, 0x90); if ((x!=0) && ((x&0x01)==0x01)) { /* Found a SBx00 base address in PCI cfg space */ i2c_amd_iobase = (int)(x & 0xffe0); } else { /* can not find a valid base address */ i2c_amd_iobase = 0; } break; default: i2c_amd_iobase = 0; break; } /* add checks for other vendors SMBUS controllers here */ } if (i2c_amd_iobase) { /* this sequence will send out addr/data on the AMD SBx00 smbus */ int timeout=DEBUG_TIMEOUT; /* check to see if the h/w is idle */ do { if (!timeout--) return; // Ran out of time. } while ((inb(i2c_amd_iobase) & 0x01)!=0x00); outb (0xFF, i2c_amd_iobase + 0); // clear error status outb (0x1F, i2c_amd_iobase + 1); // clear error status outb (data, i2c_amd_iobase + 3); // tx index outb (CONFIG_DEBUG_I2C_DEVICE_ADDR, i2c_amd_iobase + 4); // slave address and write bit outb (0x44, i2c_amd_iobase + 2); // command it to go /* check to see if the h/w is done */ do { if (!timeout--) return; // Ran out of time. } while ((inb(i2c_amd_iobase) & 0x01)!=0x00); } } // Write a character to debug port(s). static void putc_debug(struct putcinfo *action, char c) { if (! CONFIG_DEBUG_LEVEL) return; if (CONFIG_DEBUG_IO) // Send character to debug port. outb(c, GET_GLOBAL(DebugOutputPort)); if (CONFIG_DEBUG_I2C) debug_i2c(c); if (c == '\n') debug_serial('\r'); debug_serial(c); } ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Use cpu_to_be32() (and related) instead of htonl (and related).
Is there anything preventing this from being added to seabios? thanks, dave - Original Message - > From: "Dave Frodin" > To: "Kevin O'Connor" > Cc: seabios@seabios.org > Sent: Wednesday, August 15, 2012 11:18:09 AM > Subject: Re: [SeaBIOS] [PATCH] Use cpu_to_be32() (and related) instead of > htonl (and related). > > Kevin, > > This patch works for me. > > I also inappropriately ack'd the other patch "use ntohll instead of > ntohl" which > actually becomes obsolete with this patch. > > thanks, > Dave > > ----- Original Message - > > From: "Kevin O'Connor" > > To: seabios@seabios.org > > Cc: "Dave Frodin" > > Sent: Tuesday, August 14, 2012 7:20:10 PM > > Subject: [PATCH] Use cpu_to_be32() (and related) instead of htonl > > (and related). > > > > Unify the syntax for byte swab calls. > > > > This also fixes a bug in coreboot due to the lack of a > > be64_to_cpu() > > call. > > > > Signed-off-by: Kevin O'Connor > > --- > > src/ata.c | 3 ++- > > src/blockcmd.c | 17 +++--- > > src/byteorder.h | 69 > > + > > src/coreboot.c | 31 +- > > src/paravirt.c | 9 > > src/util.h | 30 - > > vgasrc/vgafb.c | 3 ++- > > 7 files changed, 103 insertions(+), 59 deletions(-) > > create mode 100644 src/byteorder.h > > > > diff --git a/src/ata.c b/src/ata.c > > index 7ff5f86..f604b37 100644 > > --- a/src/ata.c > > +++ b/src/ata.c > > @@ -8,6 +8,7 @@ > > #include "types.h" // u8 > > #include "ioport.h" // inb > > #include "util.h" // dprintf > > +#include "byteorder.h" // be16_to_cpu > > #include "cmos.h" // inb_cmos > > #include "pic.h" // enable_hwirq > > #include "biosvar.h" // GET_GLOBAL > > @@ -696,7 +697,7 @@ ata_extract_model(char *model, u32 size, u16 > > *buffer) > > // Read model name > > int i; > > for (i=0; i > -*(u16*)&model[i*2] = ntohs(buffer[27+i]); > > +*(u16*)&model[i*2] = be16_to_cpu(buffer[27+i]); > > model[size] = 0x00; > > nullTrailingSpace(model); > > return model; > > diff --git a/src/blockcmd.c b/src/blockcmd.c > > index 97c72a6..77c690f 100644 > > --- a/src/blockcmd.c > > +++ b/src/blockcmd.c > > @@ -6,7 +6,8 @@ > > // This file may be distributed under the terms of the GNU LGPLv3 > > license. > > > > #include "biosvar.h" // GET_GLOBAL > > -#include "util.h" // htonl > > +#include "util.h" // dprintf > > +#include "byteorder.h" // be32_to_cpu > > #include "disk.h" // struct disk_op_s > > #include "blockcmd.h" // struct cdb_request_sense > > #include "ata.h" // atapi_cmd_data > > @@ -144,12 +145,12 @@ scsi_init_drive(struct drive_s *drive, const > > char *s, int prio) > > // READ CAPACITY returns the address of the last block. > > // We do not bother with READ CAPACITY(16) because BIOS does > > not > > support > > // 64-bit LBA anyway. > > -drive->blksize = ntohl(capdata.blksize); > > +drive->blksize = be32_to_cpu(capdata.blksize); > > if (drive->blksize != DISK_SECTOR_SIZE) { > > dprintf(1, "%s: unsupported block size %d\n", s, > > drive->blksize); > > return -1; > > } > > -drive->sectors = (u64)ntohl(capdata.sectors) + 1; > > +drive->sectors = (u64)be32_to_cpu(capdata.sectors) + 1; > > dprintf(1, "%s blksize=%d sectors=%d\n" > > , s, drive->blksize, (unsigned)drive->sectors); > > > > @@ -243,7 +244,7 @@ cdb_mode_sense_geom(struct disk_op_s *op, > > struct > > cdbres_mode_sense_geom *data) > > cmd.command = CDB_CMD_MODE_SENSE; > > cmd.flags = 8; /* DBD */ > > cmd.page = MODE_PAGE_HD_GEOMETRY; > > -cmd.count = htons(sizeof(*data)); > > +cmd.count = cpu_to_be16(sizeof(*data)); > > op->count = 1; > > op->buf_fl = data; > > return cdb_cmd_data(op, &cmd, sizeof(*data)); > > @@ -256,8 +257,8 @@ cdb_read(struct disk_op_s *op) > > struct cdb_rwdata_10 cmd; > > memset(&cmd, 0, sizeof(cmd)); > > cmd.command = CDB_CMD_READ_10; > > -
[SeaBIOS] Bootorder file question regarding USB hubs/devices
Previously (before fetching the latest seabios/master) our bootorder file looked like this /pci@i0cf8/usb@12,2/*@4 /pci@i0cf8/usb@12,2/*@5 /pci@i0cf8/usb@12,2/*@3 /pci@i0cf8/usb@12,2/*@2 /pci@i0cf8/usb@12,2/*@1 /pci@i0cf8/usb@12,2/*@0 Now it looks like this. This also includes devices plugged into hubs on two of the ports. (Thank you to whoever got hubs working) /pci@i0cf8/usb@12,2/storage@5/*@0/*@0,0 /pci@i0cf8/usb@12,2/storage@4/*@0/*@0,0 /pci@i0cf8/usb@12,2/storage@3/*@0/*@0,0 /pci@i0cf8/usb@12,2/storage@2/*@0/*@0,0 /pci@i0cf8/usb@12,2/storage@1/*@0/*@0,0 /pci@i0cf8/usb@12,2/storage@0/*@0/*@0,0 /pci@i0cf8/usb@12,2/hub@4/storage@1/*@0/*@0,0 /pci@i0cf8/usb@12,2/hub@4/storage@2/*@0/*@0,0 /pci@i0cf8/usb@12,2/hub@4/storage@3/*@0/*@0,0 /pci@i0cf8/usb@12,2/hub@4/storage@4/*@0/*@0,0 /pci@i0cf8/usb@12,2/hub@1/storage@1/*@0/*@0,0 /pci@i0cf8/usb@12,2/hub@1/storage@2/*@0/*@0,0 /pci@i0cf8/usb@12,2/hub@1/storage@3/*@0/*@0,0 /pci@i0cf8/usb@12,2/hub@1/storage@4/*@0/*@0,0 Is there an easier/generic way to condense this down? Is there any kind of wildcard support when it comes to adding any USB device? Thanks in advance, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] What do I pass to cbfs_run_payload() after finding the payload with romfile_find() ?
> Subject: Re: [SeaBIOS] What do I pass to cbfs_run_payload() after finding the > payload with romfile_find() ? > > On Wed, Aug 29, 2012 at 02:06:03PM -0500, Dave Frodin wrote: > [...] > > This is where I'm at after the change ... > > > > // Output the LCD splash image to the Explorer board > > struct romfile_s *file; > > dprintf(1,"Looking for Explorer LCD splash payload ... "); > > file = romfile_find("img/explorer-splash"); > > if(file) > > { > > dprintf(1," found file [%s]. Loading it...\n ",file->name); <=== > > everything works up to here > > // cbfs_run_payload(); > > } > > else dprintf(1,"could NOT find it!\n"); > > > > The cbfs_run_payload is expecting (cbfs_file *). How do I extract > > what cbfs_run_payload() wants from what > > I got back from romfile_find() ? > > cbfs_run_payload((void*)file->id); > > -Kevin > Kevin, Well that was easy enough. Thanks, again Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] What do I pass to cbfs_run_payload() after finding the payload with romfile_find() ?
I'm having a problem finding/loading/executing a payload from seabios which is now using the romfile functions for cbfs (commit 59d6ca52a7eba5). I do have the cpu_to_be32() patch to get the correct payload destination address. Here's what I had previously that worked before the change... // Output the LCD splash image to the Explorer board struct cbfs_file *file; dprintf(1,"Looking for Explorer LCD splash payload ... "); file = cbfs_finddatafile("img/explorer-splash"); if(file) { dprintf(1," found file [%s]. Loading it...\n ",file->filename); cbfs_run_payload(file); } else dprintf(1,"could NOT find it!\n"); This is where I'm at after the change ... // Output the LCD splash image to the Explorer board struct romfile_s *file; dprintf(1,"Looking for Explorer LCD splash payload ... "); file = romfile_find("img/explorer-splash"); if(file) { dprintf(1," found file [%s]. Loading it...\n ",file->name); <=== everything works up to here // cbfs_run_payload(); } else dprintf(1,"could NOT find it!\n"); The cbfs_run_payload is expecting (cbfs_file *). How do I extract what cbfs_run_payload() wants from what I got back from romfile_find() ? Thanks in advance, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Use cpu_to_be32() (and related) instead of htonl (and related).
Kevin, This patch works for me. I also inappropriately ack'd the other patch "use ntohll instead of ntohl" which actually becomes obsolete with this patch. thanks, Dave - Original Message - > From: "Kevin O'Connor" > To: seabios@seabios.org > Cc: "Dave Frodin" > Sent: Tuesday, August 14, 2012 7:20:10 PM > Subject: [PATCH] Use cpu_to_be32() (and related) instead of htonl (and > related). > > Unify the syntax for byte swab calls. > > This also fixes a bug in coreboot due to the lack of a be64_to_cpu() > call. > > Signed-off-by: Kevin O'Connor > --- > src/ata.c | 3 ++- > src/blockcmd.c | 17 +++--- > src/byteorder.h | 69 > + > src/coreboot.c | 31 +- > src/paravirt.c | 9 > src/util.h | 30 - > vgasrc/vgafb.c | 3 ++- > 7 files changed, 103 insertions(+), 59 deletions(-) > create mode 100644 src/byteorder.h > > diff --git a/src/ata.c b/src/ata.c > index 7ff5f86..f604b37 100644 > --- a/src/ata.c > +++ b/src/ata.c > @@ -8,6 +8,7 @@ > #include "types.h" // u8 > #include "ioport.h" // inb > #include "util.h" // dprintf > +#include "byteorder.h" // be16_to_cpu > #include "cmos.h" // inb_cmos > #include "pic.h" // enable_hwirq > #include "biosvar.h" // GET_GLOBAL > @@ -696,7 +697,7 @@ ata_extract_model(char *model, u32 size, u16 > *buffer) > // Read model name > int i; > for (i=0; i -*(u16*)&model[i*2] = ntohs(buffer[27+i]); > +*(u16*)&model[i*2] = be16_to_cpu(buffer[27+i]); > model[size] = 0x00; > nullTrailingSpace(model); > return model; > diff --git a/src/blockcmd.c b/src/blockcmd.c > index 97c72a6..77c690f 100644 > --- a/src/blockcmd.c > +++ b/src/blockcmd.c > @@ -6,7 +6,8 @@ > // This file may be distributed under the terms of the GNU LGPLv3 > license. > > #include "biosvar.h" // GET_GLOBAL > -#include "util.h" // htonl > +#include "util.h" // dprintf > +#include "byteorder.h" // be32_to_cpu > #include "disk.h" // struct disk_op_s > #include "blockcmd.h" // struct cdb_request_sense > #include "ata.h" // atapi_cmd_data > @@ -144,12 +145,12 @@ scsi_init_drive(struct drive_s *drive, const > char *s, int prio) > // READ CAPACITY returns the address of the last block. > // We do not bother with READ CAPACITY(16) because BIOS does not > support > // 64-bit LBA anyway. > -drive->blksize = ntohl(capdata.blksize); > +drive->blksize = be32_to_cpu(capdata.blksize); > if (drive->blksize != DISK_SECTOR_SIZE) { > dprintf(1, "%s: unsupported block size %d\n", s, > drive->blksize); > return -1; > } > -drive->sectors = (u64)ntohl(capdata.sectors) + 1; > +drive->sectors = (u64)be32_to_cpu(capdata.sectors) + 1; > dprintf(1, "%s blksize=%d sectors=%d\n" > , s, drive->blksize, (unsigned)drive->sectors); > > @@ -243,7 +244,7 @@ cdb_mode_sense_geom(struct disk_op_s *op, struct > cdbres_mode_sense_geom *data) > cmd.command = CDB_CMD_MODE_SENSE; > cmd.flags = 8; /* DBD */ > cmd.page = MODE_PAGE_HD_GEOMETRY; > -cmd.count = htons(sizeof(*data)); > +cmd.count = cpu_to_be16(sizeof(*data)); > op->count = 1; > op->buf_fl = data; > return cdb_cmd_data(op, &cmd, sizeof(*data)); > @@ -256,8 +257,8 @@ cdb_read(struct disk_op_s *op) > struct cdb_rwdata_10 cmd; > memset(&cmd, 0, sizeof(cmd)); > cmd.command = CDB_CMD_READ_10; > -cmd.lba = htonl(op->lba); > -cmd.count = htons(op->count); > +cmd.lba = cpu_to_be32(op->lba); > +cmd.count = cpu_to_be16(op->count); > return cdb_cmd_data(op, &cmd, GET_GLOBAL(op->drive_g->blksize)); > } > > @@ -268,7 +269,7 @@ cdb_write(struct disk_op_s *op) > struct cdb_rwdata_10 cmd; > memset(&cmd, 0, sizeof(cmd)); > cmd.command = CDB_CMD_WRITE_10; > -cmd.lba = htonl(op->lba); > -cmd.count = htons(op->count); > +cmd.lba = cpu_to_be32(op->lba); > +cmd.count = cpu_to_be16(op->count); > return cdb_cmd_data(op, &cmd, GET_GLOBAL(op->drive_g->blksize)); > } > diff --git a/src/byteorder.h b/src/byteorder.h > new file mode 100644 > index 000..94e3a3b > --- /dev/null > +++ b/src/byteorder.h > @@ -0,0 +1,69 @@ > +#ifndef __BYTEORDER_H > +#define
Re: [SeaBIOS] The cbfs header for a payloads dest addr is a u64, use ntohll instead of ntohl
Kevin, Thanks, your patch works for me. Dave - Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" > Cc: "Peter Stuge" , seabios@seabios.org > Sent: Tuesday, August 14, 2012 7:20:01 PM > Subject: Re: [SeaBIOS] The cbfs header for a payloads dest addr is a u64, use > ntohll instead of ntohl > > On Wed, Aug 08, 2012 at 12:07:50PM -0600, Dave Frodin wrote: > > Peter, > > > > You're right, that u64 cast wasn't needed, but I needed to cast the > > result to u32. > > The (void*) is evidently expecting a u32. Is that correct? Would > > SeaBIOS always be > > expecting a u32 for an address to execute from? > > > > I also cleaned up the macro to use the existing ntohl. > > The new patch is below. I also tested it. > > Thanks for catching this. I think it would be worthwhile to clean up > the byteorder calls - see a patch I've sent in a separate email. > > -Kevin > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] The cbfs header for a payloads dest addr is a u64, use ntohll instead of ntohl
Peter, Have you had a chance to review my last patch (below)? thanks, dave - Original Message - > From: "Dave Frodin" > To: "Peter Stuge" > Cc: seabios@seabios.org > Sent: Wednesday, August 8, 2012 12:07:50 PM > Subject: Re: [SeaBIOS] The cbfs header for a payloads dest addr is a u64, > use ntohll instead of ntohl > > Peter, > > You're right, that u64 cast wasn't needed, but I needed to cast the > result to u32. > The (void*) is evidently expecting a u32. Is that correct? Would > SeaBIOS always be > expecting a u32 for an address to execute from? > > I also cleaned up the macro to use the existing ntohl. > The new patch is below. I also tested it. > > dave > > commit 66c82fdbf283340067a8531ef6e3afae82102396 > Author: Dave Frodin > Date: Tue Aug 7 17:01:08 2012 -0600 > > Seabios: This fixes reading of CBFS 64 bit destination addresses > from payload headers. > This allows img/payloads to run. > > diff --git a/src/coreboot.c b/src/coreboot.c > index e116a14..b989517 100644 > --- a/src/coreboot.c > +++ b/src/coreboot.c > @@ -470,6 +470,8 @@ struct cbfs_payload { > struct cbfs_payload_segment segments[1]; > }; > > +#define ntohll(in) (((u64) ntohl( (in) & 0x) << 32) | ((u64) > ntohl( (in) >> 32))) > + > void > cbfs_run_payload(struct cbfs_file *file) > { > @@ -480,7 +482,7 @@ cbfs_run_payload(struct cbfs_file *file) > struct cbfs_payload_segment *seg = pay->segments; > for (;;) { > void *src = (void*)pay + ntohl(seg->offset); > -void *dest = (void*)ntohl((u32)seg->load_addr); > +void *dest = (void*)(u32)ntohll(seg->load_addr); > u32 src_len = ntohl(seg->len); > u32 dest_len = ntohl(seg->mem_len); > switch (seg->type) { > > > > > - Original Message - > > From: "Peter Stuge" > > To: seabios@seabios.org > > Sent: Wednesday, August 8, 2012 5:29:57 AM > > Subject: Re: [SeaBIOS] The cbfs header for a payloads dest addr is > > a u64, use ntohll instead of ntohl > > > > Dave Frodin wrote: > > > @@ -480,7 +491,7 @@ cbfs_run_payload(struct cbfs_file *file) > > > struct cbfs_payload_segment *seg = pay->segments; > > > for (;;) { > > > void *src = (void*)pay + ntohl(seg->offset); > > > -void *dest = (void*)ntohl((u32)seg->load_addr); > > > +void *dest = (void*)ntohll((u64)seg->load_addr); > > > > Is the (u64) cast still needed? The less casts the better. > > > > > > //Peter > > > > ___ > > SeaBIOS mailing list > > SeaBIOS@seabios.org > > http://www.seabios.org/mailman/listinfo/seabios > > > > ___ > SeaBIOS mailing list > SeaBIOS@seabios.org > http://www.seabios.org/mailman/listinfo/seabios > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] The cbfs header for a payloads dest addr is a u64, use ntohll instead of ntohl
Peter, You're right, that u64 cast wasn't needed, but I needed to cast the result to u32. The (void*) is evidently expecting a u32. Is that correct? Would SeaBIOS always be expecting a u32 for an address to execute from? I also cleaned up the macro to use the existing ntohl. The new patch is below. I also tested it. dave commit 66c82fdbf283340067a8531ef6e3afae82102396 Author: Dave Frodin Date: Tue Aug 7 17:01:08 2012 -0600 Seabios: This fixes reading of CBFS 64 bit destination addresses from payload headers. This allows img/payloads to run. diff --git a/src/coreboot.c b/src/coreboot.c index e116a14..b989517 100644 --- a/src/coreboot.c +++ b/src/coreboot.c @@ -470,6 +470,8 @@ struct cbfs_payload { struct cbfs_payload_segment segments[1]; }; +#define ntohll(in) (((u64) ntohl( (in) & 0x) << 32) | ((u64) ntohl( (in) >> 32))) + void cbfs_run_payload(struct cbfs_file *file) { @@ -480,7 +482,7 @@ cbfs_run_payload(struct cbfs_file *file) struct cbfs_payload_segment *seg = pay->segments; for (;;) { void *src = (void*)pay + ntohl(seg->offset); -void *dest = (void*)ntohl((u32)seg->load_addr); +void *dest = (void*)(u32)ntohll(seg->load_addr); u32 src_len = ntohl(seg->len); u32 dest_len = ntohl(seg->mem_len); switch (seg->type) { - Original Message - > From: "Peter Stuge" > To: seabios@seabios.org > Sent: Wednesday, August 8, 2012 5:29:57 AM > Subject: Re: [SeaBIOS] The cbfs header for a payloads dest addr is a u64, > use ntohll instead of ntohl > > Dave Frodin wrote: > > @@ -480,7 +491,7 @@ cbfs_run_payload(struct cbfs_file *file) > > struct cbfs_payload_segment *seg = pay->segments; > > for (;;) { > > void *src = (void*)pay + ntohl(seg->offset); > > -void *dest = (void*)ntohl((u32)seg->load_addr); > > +void *dest = (void*)ntohll((u64)seg->load_addr); > > Is the (u64) cast still needed? The less casts the better. > > > //Peter > > ___ > SeaBIOS mailing list > SeaBIOS@seabios.org > http://www.seabios.org/mailman/listinfo/seabios > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] The cbfs header for a payloads dest addr is a u64, use ntohll instead of ntohl
All, I found an issue with seabios when it is attempting to download and execute a payload that has been added to a coreboot rom image img/ directory. The cbfs header has a destination address of "u64 load_addr". The code that reads the destination address out of the header is using ntohl which only works on u32, so the address that the cbfstool puts in the header, 0x00010 gets converted to 0x. I couldn't find where there was 64 bit version of the ntohl ftn/macro so I copied one out of coreboot. Feel free to change where/how the ntohll gets implemented. Thanks, Dave Frodin P.S. Thanks to whomever came up with the img/payload method of adding payloads. It's slick. diff --git a/src/coreboot.c b/src/coreboot.c index e116a14..4efec9c 100644 --- a/src/coreboot.c +++ b/src/coreboot.c @@ -470,6 +470,17 @@ struct cbfs_payload { struct cbfs_payload_segment segments[1]; }; +#define ntohll(x) \ + ((u64)( \ + (((u64)(x) & (u64)0x00ffULL) << 56) | \ + (((u64)(x) & (u64)0xff00ULL) << 40) | \ + (((u64)(x) & (u64)0x00ffULL) << 24) | \ + (((u64)(x) & (u64)0xff00ULL) << 8) | \ + (((u64)(x) & (u64)0x00ffULL) >> 8) | \ + (((u64)(x) & (u64)0xff00ULL) >> 24) | \ + (((u64)(x) & (u64)0x00ffULL) >> 40) | \ + (((u64)(x) & (u64)0xff00ULL) >> 56) )) + void cbfs_run_payload(struct cbfs_file *file) { @@ -480,7 +491,7 @@ cbfs_run_payload(struct cbfs_file *file) struct cbfs_payload_segment *seg = pay->segments; for (;;) { void *src = (void*)pay + ntohl(seg->offset); -void *dest = (void*)ntohl((u32)seg->load_addr); +void *dest = (void*)ntohll((u64)seg->load_addr); u32 src_len = ntohl(seg->len); u32 dest_len = ntohl(seg->mem_len); switch (seg->type) { ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] USB OHCI issues when keyboard activity occurs prior to booting from thumbdrive
Kevin, I mentioned in a response to the "Re: [SeaBIOS] usb-msc.c: TEST_UNIT_READY needs USB_DIR_OUT?" email thread that when I applied the patches related to that thread the Ubuntu booting issue that I see in this thread actually increases the passing rate from ~0% to ~50%. Perhaps setting the DIR bit twice will solve my problem :) I also was wondering if you could point me to the code that you referred to when you wrote ... "I did see what appears to be a race with USB "interrupt" pipe reading" ? thanks in advance, Dave > > > > The failure occurs when the system boots up to the GRUB menu. If I > > let the menu timeout it will boot the default option which is > > Ubuntu. If I hit the key the system will lock-up with no > > output on the debug COM port. If at the GRUB menu and I > arrow> > > to the memtest86 option, it will sometimes work and sometimes > > lockup. > > In the processes of looking through the OHCI code, I did see what > appears to be a race with USB "interrupt" pipe reading. The code is > assuming the TD is complete when the ED acknowledges the change, but > the spec is pretty clear that the TD can be modified after the ED is > updated. However, I'd be quite surprised that such a small race > would > lead to reproducible failures. I also don't see how it would relate > to EHCI thumbdrive usage. > > -Kevin > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] usb-msc.c: TEST_UNIT_READY needs USB_DIR_OUT?
I tested this (the combined patches) with several of the different USB thumbdrives that I have and didn't have any problems. In fact it actually increased the pass rate of my failing Ubuntu booting problem from ~0% to ~50%, but I'll address that in that email stream. dave - Original Message - > From: "Paolo Bonzini" > To: "Kevin O'Connor" > Cc: seabios@seabios.org > Sent: Friday, March 16, 2012 11:54:46 AM > Subject: Re: [SeaBIOS] usb-msc.c: TEST_UNIT_READY needs USB_DIR_OUT? > > Il 15/03/2012 02:31, Kevin O'Connor ha scritto: > > On Wed, Mar 14, 2012 at 01:02:39PM -0600, Steve Goodrich wrote: > >> I've been working with coreboot and SeaBIOS lately to try to get a > >> platform > >> working, including having boot capability using an SD-to-USB > >> adapter. I got > >> stuck on this last part: the USB/SD adapter would start > >> enumeration, but > >> would fail out during TEST_UNIT_READY because the device came back > >> with a > >> STALL (this via a LeCroy/CACT USB analyzer). I compared the > >> transactions to > >> those done during USB enumeration/configuration in Linux, and I > >> noticed that > >> the Direction bit in bmCBWFlags is *cleared* for the > >> TEST_UNIT_READY command > >> in Linux, but *set* in coreboot. > > > > If Linux always clears the direction bit for zero length transfers, > > then I think it should be safe to do that in SeaBIOS also. How > > about > > the patch below? > > > > -Kevin > > > > > > From 1fd9a89082b807a4bb4ab6ce1285df474cb75746 Mon Sep 17 00:00:00 > > 2001 > > From: Kevin O'Connor > > Date: Wed, 14 Mar 2012 21:27:45 -0400 > > Subject: [PATCH] Use OUT mode for all zero byte "scsi" transfers. > > > > Some devices can get confused if asked to "read" data during a zero > > byte transfer, so consider these transfers as "writes". (Reported > > by > > Steve Goodrich.) > > > > Also, extract out the code to determine the transfer direction into > > cdb_is_read(). > > > > Signed-off-by: Kevin O'Connor > > --- > > src/blockcmd.c|7 +++ > > src/blockcmd.h|1 + > > src/usb-msc.c |4 ++-- > > src/virtio-scsi.c |7 --- > > 4 files changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/src/blockcmd.c b/src/blockcmd.c > > index 8d881a5..2dda04d 100644 > > --- a/src/blockcmd.c > > +++ b/src/blockcmd.c > > @@ -35,6 +35,13 @@ cdb_cmd_data(struct disk_op_s *op, void *cdbcmd, > > u16 blocksize) > > } > > } > > > > +// Determine if the command is a request to pull data from the > > device > > +int > > +cdb_is_read(u8 *cdbcmd, u16 blocksize) > > +{ > > +return blocksize && cdbcmd[0] != CDB_CMD_WRITE_10; > > +} > > + > > int > > scsi_is_ready(struct disk_op_s *op) > > { > > diff --git a/src/blockcmd.h b/src/blockcmd.h > > index ccfeeaf..8459d3e 100644 > > --- a/src/blockcmd.h > > +++ b/src/blockcmd.h > > @@ -100,6 +100,7 @@ struct cdbres_mode_sense_geom { > > } PACKED; > > > > // blockcmd.c > > +int cdb_is_read(u8 *cdbcmd, u16 blocksize); > > int cdb_get_inquiry(struct disk_op_s *op, struct cdbres_inquiry > > *data); > > int cdb_get_sense(struct disk_op_s *op, struct > > cdbres_request_sense *data); > > int cdb_test_unit_ready(struct disk_op_s *op); > > diff --git a/src/usb-msc.c b/src/usb-msc.c > > index dadb9ca..c53a1f5 100644 > > --- a/src/usb-msc.c > > +++ b/src/usb-msc.c > > @@ -77,13 +77,13 @@ usb_cmd_data(struct disk_op_s *op, void > > *cdbcmd, u16 blocksize) > > cbw.dCBWSignature = CBW_SIGNATURE; > > cbw.dCBWTag = 999; // XXX > > cbw.dCBWDataTransferLength = bytes; > > -cbw.bmCBWFlags = (cbw.CBWCB[0] == CDB_CMD_WRITE_10) ? > > USB_DIR_OUT : USB_DIR_IN; > > +cbw.bmCBWFlags = cdb_is_read(cdbcmd, blocksize) ? USB_DIR_IN : > > USB_DIR_OUT; > > cbw.bCBWLUN = 0; // XXX > > cbw.bCBWCBLength = USB_CDB_SIZE; > > > > // Transfer cbw to device. > > int ret = usb_msc_send(udrive_g, USB_DIR_OUT > > -, MAKE_FLATPTR(GET_SEG(SS), &cbw), > > sizeof(cbw)); > > + , MAKE_FLATPTR(GET_SEG(SS), &cbw), > > sizeof(cbw)); > > if (ret) > > goto fail; > > > > diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c > > index 3c59438..50339d7 100644 > > --- a/src/virtio-scsi.c > > +++ b/src/virtio-scsi.c > > @@ -31,7 +31,7 @@ struct virtio_lun_s { > > > > static int > > virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct > > disk_op_s *op, > > -void *cdbcmd, u16 target, u16 lun, u32 len) > > +void *cdbcmd, u16 target, u16 lun, u16 blocksize) > > { > > struct virtio_scsi_req_cmd req; > > struct virtio_scsi_resp_cmd resp; > > @@ -44,7 +44,8 @@ virtio_scsi_cmd(u16 ioaddr, struct > > vring_virtqueue *vq, struct disk_op_s *op, > > req.lun[3] = (lun & 0xff); > > memcpy(req.cdb, cdbcmd, 16); > > > > -int datain = (req.cdb[0] != CDB_CMD_WRITE_10); > > +u32 len = op->count * blocksize; > > +int datain = cdb_is_read(cdbcmd, blocksize); > > The p
Re: [SeaBIOS] [PATCH 00/19] USB improvements
Tested these patches on my AMD G-series/SB800 platform (Persimmon) and didn't have any problems. I was using a USB wireless keyboard and booting Ubuntu with a USB thumbdrive. Dave - Original Message - > From: "Kevin O'Connor" > To: seabios@seabios.org > Sent: Saturday, March 10, 2012 7:41:22 PM > Subject: [SeaBIOS] [PATCH 00/19] USB improvements > > This series updates the USB code. This is mostly a code > reorganization, though there are a couple of bug fixes (mostly for > OHCI). > > -Kevin > > > Kevin O'Connor (19): > Batch free USB pipes on OHCI controllers. > Batch free USB pipes on UHCI controllers. > Batch free USB pipes on EHCI controllers. > usb: Centralize pipe free list code. > usb: Remove cntl->defaultpipe cache. > usb: Obtain free list items in main code. > usb: Introduce 'struct usbdevice_s' to describe info for a given > device. > usb: Push 'struct usbdevice_s' usage through to pipe allocation. > usb: Build path via chain of usbdevice_s. > usb: Pass usbdevice_s to alloc_async_pipe. > usb: Pass 'struct usbdevice_s' into controller alloc_async > functions. > usb: Pass 'struct usbdevice_s' to controller alloc_intr_pipe > functions. > usb: Move code around in usb controller code. > usb: Generalize controller alloc_async_pipe / alloc_intr_pipe code. > usb: Move code around in usb.c. > usb: Move EHCI tt_* fields to EHCI controller code. > usb: Populate OHCI endpoint descriptor info at pipe allocation. > usb: Fix barrier() placement in OHCI interrupt schedule add. > usb: Remove QH_MULT_SHIFT flag from qh.info1. > > src/boot.c | 25 ++-- > src/boot.h |3 +- > src/usb-ehci.c | 441 > +++- > src/usb-ehci.h |8 +- > src/usb-hid.c | 28 ++-- > src/usb-hid.h |6 +- > src/usb-hub.c | 14 +- > src/usb-hub.h |4 +- > src/usb-msc.c | 14 +- > src/usb-msc.h |6 +- > src/usb-ohci.c | 323 +- > src/usb-ohci.h |8 +- > src/usb-uhci.c | 411 > +--- > src/usb-uhci.h |8 +- > src/usb.c | 277 +-- > src/usb.h | 48 --- > 16 files changed, 797 insertions(+), 827 deletions(-) > > -- > 1.7.6.5 > > > ___ > SeaBIOS mailing list > SeaBIOS@seabios.org > http://www.seabios.org/mailman/listinfo/seabios > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 08/19] usb: Push 'struct usbdevice_s' usage through to pipe allocation.
I had to do some whitespace cleanup in order for patch 08/19 to apply cleanly. I don't have much experience in applying patches so there might be something I'm missing. Here are the changes I had to do... Subject: [PATCH] Whitespace cleanup, replace two tabs with spaces --- src/usb-msc.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/usb-msc.c b/src/usb-msc.c index e143401..62152d8 100644 --- a/src/usb-msc.c +++ b/src/usb-msc.c @@ -132,8 +132,8 @@ usb_msc_init(struct usb_pipe *pipe // Verify right kind of device if ((iface->bInterfaceSubClass != US_SC_SCSI && -iface->bInterfaceSubClass != US_SC_ATAPI_8070 && -iface->bInterfaceSubClass != US_SC_ATAPI_8020) +iface->bInterfaceSubClass != US_SC_ATAPI_8070 && +iface->bInterfaceSubClass != US_SC_ATAPI_8020) || iface->bInterfaceProtocol != US_PR_BULK) { dprintf(1, "Unsupported MSC USB device (subclass=%02x proto=%02x)\n" , iface->bInterfaceSubClass, iface->bInterfaceProtocol); -- 1.7.9 dave - Original Message - > From: "Kevin O'Connor" > To: seabios@seabios.org > Sent: Saturday, March 10, 2012 7:44:46 PM > Subject: [SeaBIOS] [PATCH 08/19] usb: Push 'struct usbdevice_s' usage through > to pipe allocation. > > Pass the usbdevice_s info to device configuration and allocation > code. > > Signed-off-by: Kevin O'Connor > --- > src/usb-hid.c | 28 +++- > src/usb-hid.h |6 ++ > src/usb-hub.c |8 > src/usb-hub.h |4 ++-- > src/usb-msc.c | 15 --- > src/usb-msc.h |6 ++ > src/usb.c | 35 +++ > src/usb.h | 13 +++-- > 8 files changed, 59 insertions(+), 56 deletions(-) > > diff --git a/src/usb-hid.c b/src/usb-hid.c > index 168b7fa..21363c3 100644 > --- a/src/usb-hid.c > +++ b/src/usb-hid.c > @@ -49,7 +49,8 @@ set_idle(struct usb_pipe *pipe, int ms) > #define KEYREPEATMS 33 > > static int > -usb_kbd_init(struct usb_pipe *pipe, struct usb_endpoint_descriptor > *epdesc) > +usb_kbd_init(struct usbdevice_s *usbdev > + , struct usb_endpoint_descriptor *epdesc) > { > if (! CONFIG_USB_KEYBOARD) > return -1; > @@ -61,15 +62,15 @@ usb_kbd_init(struct usb_pipe *pipe, struct > usb_endpoint_descriptor *epdesc) > return -1; > > // Enable "boot" protocol. > -int ret = set_protocol(pipe, 0); > +int ret = set_protocol(usbdev->defpipe, 0); > if (ret) > return -1; > // Periodically send reports to enable key repeat. > -ret = set_idle(pipe, KEYREPEATMS); > +ret = set_idle(usbdev->defpipe, KEYREPEATMS); > if (ret) > return -1; > > -keyboard_pipe = alloc_intr_pipe(pipe, epdesc); > +keyboard_pipe = alloc_intr_pipe(usbdev, epdesc); > if (!keyboard_pipe) > return -1; > > @@ -78,7 +79,8 @@ usb_kbd_init(struct usb_pipe *pipe, struct > usb_endpoint_descriptor *epdesc) > } > > static int > -usb_mouse_init(struct usb_pipe *pipe, struct usb_endpoint_descriptor > *epdesc) > +usb_mouse_init(struct usbdevice_s *usbdev > + , struct usb_endpoint_descriptor *epdesc) > { > if (! CONFIG_USB_MOUSE) > return -1; > @@ -90,11 +92,11 @@ usb_mouse_init(struct usb_pipe *pipe, struct > usb_endpoint_descriptor *epdesc) > return -1; > > // Enable "boot" protocol. > -int ret = set_protocol(pipe, 0); > +int ret = set_protocol(usbdev->defpipe, 0); > if (ret) > return -1; > > -mouse_pipe = alloc_intr_pipe(pipe, epdesc); > +mouse_pipe = alloc_intr_pipe(usbdev, epdesc); > if (!mouse_pipe) > return -1; > > @@ -104,29 +106,29 @@ usb_mouse_init(struct usb_pipe *pipe, struct > usb_endpoint_descriptor *epdesc) > > // Initialize a found USB HID device (if applicable). > int > -usb_hid_init(struct usb_pipe *pipe > - , struct usb_interface_descriptor *iface, int imax) > +usb_hid_init(struct usbdevice_s *usbdev) > { > if (! CONFIG_USB_KEYBOARD || ! CONFIG_USB_MOUSE) > return -1; > -dprintf(2, "usb_hid_init %p\n", pipe); > +dprintf(2, "usb_hid_init %p\n", usbdev->defpipe); > > +struct usb_interface_descriptor *iface = usbdev->iface; > if (iface->bInterfaceSubClass != USB_INTERFACE_SUBCLASS_BOOT) > // Doesn't support boot protocol. > return -1; > > // Find intr in endpoint. > struct usb_endpoint_descriptor *epdesc = findEndPointDesc( > -iface, imax, USB_ENDPOINT_XFER_INT, USB_DIR_IN); > +usbdev, USB_ENDPOINT_XFER_INT, USB_DIR_IN); > if (!epdesc) { > dprintf(1, "No usb hid intr in?\n"); > return -1; > } > > if (iface->bInterfaceProtocol == > USB_INTERFACE_PROTOCOL_KEYBOARD) > -return usb_kbd_init(pipe, epdesc); > +return usb_kbd_init(usbdev, epdesc); > if (iface->bInterfaceProtocol == USB_INTERFACE_PROTOCOL_MOUSE) >
[SeaBIOS] USB OHCI issues when keyboard activity occurs prior to booting from thumbdrive
All, I'm seeing another USB issue with our platform(s). I've been following the other active USB mail list issue "usb ohci pipe free fix" but don't exactly know where that one started. I'm only guessing that this could be related. The description of my h/w setup is ... - AMD/Persimmon platform (also observed failure on IEI/Kino) - USB thumbdrive with GRUB (menu options for Ubuntu,memtest86,etc) - USB wireless keyboard/mouse (also observed failure with wired USB keyboard) I using the SeaBIOS master source. The failure occurs when the system boots up to the GRUB menu. If I let the menu timeout it will boot the default option which is Ubuntu. If I hit the key the system will lock-up with no output on the debug COM port. If at the GRUB menu and I to the memtest86 option, it will sometimes work and sometimes lockup. I then changed the SeaBIOS debug message level to 8. Then at the point where I hit for the default GRUB option. I will see several debug messages indicating traffic related to the USB thumbdrive and then never ending "handle_hwpic1 irq=0" messages. Then as I was browsing through the usb code I noticed that some of the keyboard messages only occur at level 9, so I set the level to 9 and all of my booting failures went away. ?. I tracked the message that was somehow fixing my problem down to the file usb-hid.c, in ftn handle_key() (~line 216). If I change that "dprintf(9,..." to "dprintf(1,...", and change the debug message level down below 9, everything still works. So the delay due to that one debug message fixes some issue related to using a USB keyboard and thumbdrive at the same time. I don't see this issue if I use a SATA drive instead of the USB thumbdrive. Our in-house USB guru (whose not familiar with coreboot) suggests it could be an issue with the OHCI stack. Does anybody have any suggestions on what this could be? Or where I should start looking? thanks in advance, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] scsi: do not send MODE SENSE except to QEMU disks
Signed-off-by: Dave Frodin Paolo, Marc wanted me to request that this patch be included in the next stable SeaBIOS release. Thanks again for your help on this. Dave - Original Message - > From: "Paolo Bonzini" > To: seabios@seabios.org > Cc: "Dave Frodin" > Sent: Monday, March 5, 2012 4:29:12 AM > Subject: [PATCH] scsi: do not send MODE SENSE except to QEMU disks > > This is the simplest way to avoid breaking boot on USB sticks that > stall when asked for the MODE SENSE page 4. Some old sticks do > not support the MODE SENSE command at all and just return a > "medium may have changed" unit attention condition when SeaBIOS > sends it! > > Reported-by: Dave Frodin > Signed-off-by: Paolo Bonzini > --- > src/blockcmd.c | 36 +++- > 1 files changed, 23 insertions(+), 13 deletions(-) > > diff --git a/src/blockcmd.c b/src/blockcmd.c > index 2769573..b4a1e37 100644 > --- a/src/blockcmd.c > +++ b/src/blockcmd.c > @@ -139,19 +139,29 @@ scsi_init_drive(struct drive_s *drive, const > char *s, int prio) > dprintf(1, "%s blksize=%d sectors=%d\n" > , s, drive->blksize, (unsigned)drive->sectors); > > -struct cdbres_mode_sense_geom geomdata; > -ret = cdb_mode_sense_geom(&dop, &geomdata); > -if (ret == 0) { > -u32 cylinders; > -cylinders = geomdata.cyl[0] << 16; > -cylinders |= geomdata.cyl[1] << 8; > -cylinders |= geomdata.cyl[2]; > -if (cylinders && geomdata.heads && > -drive->sectors <= 0xULL && > -((u32)drive->sectors % (geomdata.heads * cylinders) == > 0)) { > -drive->pchs.cylinders = cylinders; > -drive->pchs.heads = geomdata.heads; > -drive->pchs.spt = (u32)drive->sectors / (geomdata.heads > * cylinders); > +// We do not recover from USB stalls, so try to be safe and > avoid > +// sending the command if the (obsolete, but still provided by > QEMU) > +// fixed disk geometry page may not be supported. > +// > +// We could also send the command only to small disks (e.g. > <504MiB) > +// but some old USB keys only support a very small subset of > SCSI which > +// does not even include the MODE SENSE command! > +// > +if (! CONFIG_COREBOOT && memcmp(vendor, "QEMU", 8) == 0) { > +struct cdbres_mode_sense_geom geomdata; > +ret = cdb_mode_sense_geom(&dop, &geomdata); > +if (ret == 0) { > +u32 cylinders; > +cylinders = geomdata.cyl[0] << 16; > +cylinders |= geomdata.cyl[1] << 8; > +cylinders |= geomdata.cyl[2]; > +if (cylinders && geomdata.heads && > +drive->sectors <= 0xULL && > +((u32)drive->sectors % (geomdata.heads * cylinders) > == 0)) { > +drive->pchs.cylinders = cylinders; > +drive->pchs.heads = geomdata.heads; > +drive->pchs.spt = (u32)drive->sectors / > (geomdata.heads * cylinders); > +} > } > } > > -- > 1.7.7.6 > > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] scsi: do not send MODE SENSE except to QEMU disks
Paolo, I tested your patch. It works with all 14 USB thumbdrives and with a USB-to-SATA adapter we dug up. Dave - Original Message - > From: "Paolo Bonzini" > To: seabios@seabios.org > Cc: "Dave Frodin" > Sent: Monday, March 5, 2012 4:29:12 AM > Subject: [PATCH] scsi: do not send MODE SENSE except to QEMU disks > > This is the simplest way to avoid breaking boot on USB sticks that > stall when asked for the MODE SENSE page 4. Some old sticks do > not support the MODE SENSE command at all and just return a > "medium may have changed" unit attention condition when SeaBIOS > sends it! > > Reported-by: Dave Frodin > Signed-off-by: Paolo Bonzini > --- > src/blockcmd.c | 36 +++- > 1 files changed, 23 insertions(+), 13 deletions(-) > > diff --git a/src/blockcmd.c b/src/blockcmd.c > index 2769573..b4a1e37 100644 > --- a/src/blockcmd.c > +++ b/src/blockcmd.c > @@ -139,19 +139,29 @@ scsi_init_drive(struct drive_s *drive, const > char *s, int prio) > dprintf(1, "%s blksize=%d sectors=%d\n" > , s, drive->blksize, (unsigned)drive->sectors); > > -struct cdbres_mode_sense_geom geomdata; > -ret = cdb_mode_sense_geom(&dop, &geomdata); > -if (ret == 0) { > -u32 cylinders; > -cylinders = geomdata.cyl[0] << 16; > -cylinders |= geomdata.cyl[1] << 8; > -cylinders |= geomdata.cyl[2]; > -if (cylinders && geomdata.heads && > -drive->sectors <= 0xULL && > -((u32)drive->sectors % (geomdata.heads * cylinders) == > 0)) { > -drive->pchs.cylinders = cylinders; > -drive->pchs.heads = geomdata.heads; > -drive->pchs.spt = (u32)drive->sectors / (geomdata.heads > * cylinders); > +// We do not recover from USB stalls, so try to be safe and > avoid > +// sending the command if the (obsolete, but still provided by > QEMU) > +// fixed disk geometry page may not be supported. > +// > +// We could also send the command only to small disks (e.g. > <504MiB) > +// but some old USB keys only support a very small subset of > SCSI which > +// does not even include the MODE SENSE command! > +// > +if (! CONFIG_COREBOOT && memcmp(vendor, "QEMU", 8) == 0) { > +struct cdbres_mode_sense_geom geomdata; > +ret = cdb_mode_sense_geom(&dop, &geomdata); > +if (ret == 0) { > +u32 cylinders; > +cylinders = geomdata.cyl[0] << 16; > +cylinders |= geomdata.cyl[1] << 8; > +cylinders |= geomdata.cyl[2]; > +if (cylinders && geomdata.heads && > +drive->sectors <= 0xULL && > +((u32)drive->sectors % (geomdata.heads * cylinders) > == 0)) { > +drive->pchs.cylinders = cylinders; > +drive->pchs.heads = geomdata.heads; > +drive->pchs.spt = (u32)drive->sectors / > (geomdata.heads * cylinders); > +} > } > } > > -- > 1.7.7.6 > > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Booting from USB thumbdrives, older drives boot, newer drives don't.
- Original Message - > From: "Paolo Bonzini" > To: "Kevin O'Connor" > Cc: "Dave Frodin" , seabios@seabios.org > Sent: Monday, March 5, 2012 1:17:08 AM > Subject: Re: Booting from USB thumbdrives, older drives boot, newer drives > don't. > > Il 04/03/2012 19:49, Kevin O'Connor ha scritto: > >> > The sg_modes command has a "-a" option to dump out all of the > >> > supported page codes > >> >- it reports that none of the thumbdrives (tested 9) support > >> >page code = 4 > >> >- it reports that most of the thumbdrives (tested 9) support > >> >page code = 5 > >> >- it commented that page code 5 is obsolete > > Thanks Dave. I appreciate your detailed analysis. > > > > The call to cdb_mode_sense_geom() was only recently added. I'm > > inclined to just remove the call. Paolo, is there a use case where > > having the "physical" chs info is important? SeaBIOS should > > generate > > good "virtual" chs info regardless, and the underlying "physical" > > info > > is only exported in a couple of places. > > MS-DOS doesn't boot if you do not export the physical chs. I can > remove > it, but it was the best testcase I had for SCSI support. > > The simplest solution seems to be to look for page 4 support in page > 0x3f. Except that none of the 12 devices I've tested support page 4. There's also a good chance that some of the devices could generate the stall on the page 3f request. That's what led us to thinking we should leave the page 4 query in and just add support to clear the stall. > > Paolo > > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Booting from USB thumbdrives, older drives boot, newer drives don't.
Kevin/Paolo, Our concern was that actual rotating media (e.g. USB-to-SATA adapter) may have problems without the cdb_mode_sense_geom(). I hope to have my hands on one tomorrow to do some testing. Dave - Original Message - > From: "Kevin O'Connor" > To: "Dave Frodin" , "Paolo Bonzini" > > Cc: seabios@seabios.org > Sent: Sunday, March 4, 2012 11:49:30 AM > Subject: Re: [SeaBIOS] Booting from USB thumbdrives, older drives boot, newer > drives don't. > > On Fri, Mar 02, 2012 at 03:10:33PM -0700, Dave Frodin wrote: > > Here are my latest results. > > The sg_modes command has a "-a" option to dump out all of the > > supported page codes > >- it reports that none of the thumbdrives (tested 9) support > >page code = 4 > >- it reports that most of the thumbdrives (tested 9) support > >page code = 5 > >- it commented that page code 5 is obsolete > > Thanks Dave. I appreciate your detailed analysis. > > The call to cdb_mode_sense_geom() was only recently added. I'm > inclined to just remove the call. Paolo, is there a use case where > having the "physical" chs info is important? SeaBIOS should generate > good "virtual" chs info regardless, and the underlying "physical" > info > is only exported in a couple of places. > > > Marc suggested that I leave the page_code set to 4 in order to > > preserve functionality for drives that actually report valid CHS > > geometry, and to add code that would clear the stall from the USB > > controller and thumbdrive. > > The SeaBIOS code really should know how to clear a stall. However, > it's a bit of a pain to do that in the general case. (In the general > case, we'd need to support clearing the stall from 16bit mode, which > would require the ability to send control messages to the drive in > 16bit mode.) > > > 2) Is there any way to differentiate between a USB thumbdrive and > > a USB-to-SATA adapter? > > I don't know of a way to detect it, but I haven't looked closely. > > -Kevin > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Booting from USB thumbdrives, older drives boot, newer drives don't.
Here are my latest results. The sg_modes command has a "-a" option to dump out all of the supported page codes - it reports that none of the thumbdrives (tested 9) support page code = 4 - it reports that most of the thumbdrives (tested 9) support page code = 5 - it commented that page code 5 is obsolete the page_code=4 and 5 cyc/head info must be garbage these are the types of values I get back. I haven't found a spec yet for the page_code 5 geometry struct so it could be bogus because of that. cyc[0]/cyc[1]/cyc[2] head --- page_code 4 page_code 5 old_sandisk 00/00/00 0 80/00/04 20 new_sandisk stall00/00/10 3F verbatim 66/b5/02 f f0/00/ff 20 wintec00/00/80 0 13/88/10 3F I think the booting drives boot because ... A) using page_code=5 got rid of the stall (even though the data is bogus) B) the MBR is at sector 0, and 0 is 0 no matter what the geometry C) perhaps there is code later in seabios that understands it's booting from a USB drive and knows better than us what to do and uses something like linear sectors. Marc suggested that I leave the page_code set to 4 in order to preserve functionality for drives that actually report valid CHS geometry, and to add code that would clear the stall from the USB controller and thumbdrive. Questions: 1) Any recommendations on my direction? 2) Is there any way to differentiate between a USB thumbdrive and a USB-to-SATA adapter? dave - Original Message - > From: "Dave Frodin" > To: seabios@seabios.org > Sent: Friday, March 2, 2012 8:53:54 AM > Subject: Re: [SeaBIOS] Booting from USB thumbdrives, older drives boot, > newer drives don't. > > Paolo, > Thanks for the reply. > I ran the sg_modes for pages 4 and 5 on 4 USB thumbdrives. > sandisk_old is a 4GB Sandisk Cruzer (yes, the old is ver=8.02) > sandisk_new is a 4GB Sandisk Cruzer > transcend is a 2GB drive > verbatimis a 2GB drive > > If I use the unmodified "master" SeaBIOS ... > sandisk_old boots > sandisk_new stalls at mode sense > transcend stalls at mode sense > verbatim boots > > If I change the mode page from 4 to 5 ... > sandisk_old boots > sandisk_new boots > transcend stalls at mode sense > verbatim boots > > The mode page change from 4 to 5 allowed four of my none booting > drives to boot. Hopefully these logs will be helpful. > > Thanks again, > dave > > starting sandisk_old tests >sudo sg_modes -HHp 4 /dev/sdb > SanDisk Cruzer8.02 peripheral_type: disk [0x0] > Mode parameter header from MODE SENSE(10): > 00 00 46 00 00 00 00 00 00 > Mode data length=72, medium type=0x00, WP=0, DpoFua=0, longlba=0 > Block descriptor length=0 > >> page_code=0x0, page_control=0 > 00 00 00 > Unexpectedly received extra mode page responses, ignore > >sudo sg_modes -HHp 5 /dev/sdb > SanDisk Cruzer8.02 peripheral_type: disk [0x0] > Mode parameter header from MODE SENSE(10): > 00 00 26 00 00 00 00 00 00 > Mode data length=40, medium type=0x00, WP=0, DpoFua=0, longlba=0 > Block descriptor length=0 > >> page_code=0x5, page_control=0 > 00 05 1e 80 00 04 20 02 00 ef c0 00 00 00 00 00 00 > 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > starting sandisk_new tests >sudo sg_modes -HHp 4 /dev/sdb > SanDisk Cruzer 1.26 peripheral_type: disk [0x0] > invalid field in cdb (perhaps page 0x4 not supported) > >sudo sg_modes -HHp 5 /dev/sdb > SanDisk Cruzer 1.26 peripheral_type: disk [0x0] > Mode parameter header from MODE SENSE(10): > 00 00 26 00 00 00 00 00 00 > Mode data length=40, medium type=0x00, WP=0, DpoFua=0, longlba=0 > Block descriptor length=0 > >> page_code=0x5, page_control=0 > 00 05 1e 00 00 10 3f 02 00 1e 4f 00 00 00 00 00 00 > 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > starting transcend >sudo sg_modes -HHp 4 /dev/sdb > JetFlash Transcend 2GB 8.07 peripheral_type: disk [0x0] > Mode parameter header from MODE SENSE(10): > 00 00 06 00 00 00 00 00 00 > Mode data length=8, medium type=0x00, WP=0, DpoFua=0, longlba=0 > Block descriptor length=0 > >sudo sg_modes -HHp 5 /dev/sdb > JetFlash Transcend 2GB 8.07 peripheral_type: disk [0x0] > Mode parameter hea
Re: [SeaBIOS] Booting from USB thumbdrives, older drives boot, newer drives don't.
Paolo, Thanks for the reply. I ran the sg_modes for pages 4 and 5 on 4 USB thumbdrives. sandisk_old is a 4GB Sandisk Cruzer (yes, the old is ver=8.02) sandisk_new is a 4GB Sandisk Cruzer transcend is a 2GB drive verbatimis a 2GB drive If I use the unmodified "master" SeaBIOS ... sandisk_old boots sandisk_new stalls at mode sense transcend stalls at mode sense verbatim boots If I change the mode page from 4 to 5 ... sandisk_old boots sandisk_new boots transcend stalls at mode sense verbatim boots The mode page change from 4 to 5 allowed four of my none booting drives to boot. Hopefully these logs will be helpful. Thanks again, dave starting sandisk_old tests sudo sg_modes -HHp 4 /dev/sdb SanDisk Cruzer8.02 peripheral_type: disk [0x0] Mode parameter header from MODE SENSE(10): 00 00 46 00 00 00 00 00 00 Mode data length=72, medium type=0x00, WP=0, DpoFua=0, longlba=0 Block descriptor length=0 >> page_code=0x0, page_control=0 00 00 00 Unexpectedly received extra mode page responses, ignore sudo sg_modes -HHp 5 /dev/sdb SanDisk Cruzer8.02 peripheral_type: disk [0x0] Mode parameter header from MODE SENSE(10): 00 00 26 00 00 00 00 00 00 Mode data length=40, medium type=0x00, WP=0, DpoFua=0, longlba=0 Block descriptor length=0 >> page_code=0x5, page_control=0 00 05 1e 80 00 04 20 02 00 ef c0 00 00 00 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 starting sandisk_new tests sudo sg_modes -HHp 4 /dev/sdb SanDisk Cruzer 1.26 peripheral_type: disk [0x0] invalid field in cdb (perhaps page 0x4 not supported) sudo sg_modes -HHp 5 /dev/sdb SanDisk Cruzer 1.26 peripheral_type: disk [0x0] Mode parameter header from MODE SENSE(10): 00 00 26 00 00 00 00 00 00 Mode data length=40, medium type=0x00, WP=0, DpoFua=0, longlba=0 Block descriptor length=0 >> page_code=0x5, page_control=0 00 05 1e 00 00 10 3f 02 00 1e 4f 00 00 00 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 starting transcend sudo sg_modes -HHp 4 /dev/sdb JetFlash Transcend 2GB 8.07 peripheral_type: disk [0x0] Mode parameter header from MODE SENSE(10): 00 00 06 00 00 00 00 00 00 Mode data length=8, medium type=0x00, WP=0, DpoFua=0, longlba=0 Block descriptor length=0 sudo sg_modes -HHp 5 /dev/sdb JetFlash Transcend 2GB 8.07 peripheral_type: disk [0x0] Mode parameter header from MODE SENSE(10): 00 00 06 00 00 00 00 00 00 Mode data length=8, medium type=0x00, WP=0, DpoFua=0, longlba=0 Block descriptor length=0 starting verbatim sudo sg_modes -HHp 4 /dev/sdb Verbatim STORE N GO5.00 peripheral_type: disk [0x0] Mode parameter header from MODE SENSE(10): 00 00 06 00 00 00 00 00 00 Mode data length=8, medium type=0x00, WP=0, DpoFua=0, longlba=0 Block descriptor length=0 sudo sg_modes -HHp 5 /dev/sdb Verbatim STORE N GO5.00 peripheral_type: disk [0x0] Mode parameter header from MODE SENSE(10): 00 00 26 00 00 00 00 00 00 Mode data length=40, medium type=0x00, WP=0, DpoFua=0, longlba=0 Block descriptor length=0 >> page_code=0x5, page_control=0 00 05 1e f0 00 ff 20 02 00 01 df 00 00 00 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 - Original Message - > From: "Paolo Bonzini" > To: seabios@seabios.org, d...@camp.se-eng.com > Sent: Friday, March 2, 2012 7:20:28 AM > Subject: Re: Booting from USB thumbdrives, older drives boot, newer drives > don't. > > Il 02/03/2012 01:34, Dave Frodin ha scritto: > > On a related topic, I have a copy of the "Indispensable PC > > Hardware" book > > that lists the return values for the mode page 4 request. It list > > the cyc/head > > data as follows... > > offset value > > == > >2 cyl(msb) > >3 cyl > >4 cyl(lsb) > >5 heads > > > > the struct in the blockcmd.h file shows... > > struct cdbres_mode_sense_geom { > > u8 unused_00[3]; > > u8 read_only; > > u32 unused_04; > > u8 page; &g
Re: [SeaBIOS] Booting from USB thumbdrives, older drives boot, newer drives don't.
I have some addition results and questions. I changed the "page mode" value that is in the struct sent in the MODE SENSE command from the existing value of 04h (rigid disk) to 05h (flexible disk). This change increased the number of passing USB thumbdrives from 6 to 10. I'm testing with a total of 12 different vendor/size drives. The two failing drives still send a USB stall but the failure mode is different. With the page mode type set to 4 the failure sequence was ... 1) send MODE SENSE command to drive => device accepts data and ACKs 2) device stalls the USB bus With the page mode set to 5 the failure sequence is ... 1) send MODE SENSE command to drive => device accepts data and ACKs 2) device sends response => host accepts data and ACKs 3) device stalls the USB bus Also, the device only sends 6 bytes instead of the requested 27 bytes. I'm planning on seeing if there isn't something missing prior to the mode sense command tomorrow. On a related topic, I have a copy of the "Indispensable PC Hardware" book that lists the return values for the mode page 4 request. It list the cyc/head data as follows... offset value == 2 cyl(msb) 3 cyl 4 cyl(lsb) 5 heads the struct in the blockcmd.h file shows... struct cdbres_mode_sense_geom { u8 unused_00[3]; u8 read_only; u32 unused_04; u8 page; u8 length; u8 cyl[3]; u8 heads; u8 precomp[3]; u8 reduced[3]; u16 step_rate; u8 landing[3]; u16 rpm; } PACKED; which would put cyl[3] at offset 10 thru 12 and heads at offset 13. Am I missing something here? USB thumbdrives that don't stall on the mode sense return values like cyl = 10h 3Fh 02h heads = 00h ??? The values for that drive have 00h at offsets 2 thru 5. I'm looking forward to any insight into this from anyone knowledgeable on the subject. Dave - Original Message - > From: "Dave Frodin" > To: seabios@seabios.org > Sent: Wednesday, February 29, 2012 4:38:21 PM > Subject: [SeaBIOS] Booting from USB thumbdrives, older drives boot, newer > drives don't. > > I'm a new subscriber to seabios.org so feel free to straighten me out > if needed. > > I've been debugging an problem we've been seeing with not being able > to boot (Ubuntu specifically) off > of a variety of "newer" USB thumb drives. I've been specifically > looking at an older/newer pair of > Sandisk Cruzer 4GB drives. I've been adding dprintf's to narrow down > the problem to the blockcmd.c file. > The function scsi_init_drive() queries the USB device to determine > stuff like vendor/device/size/etc. > Near the end of the function is a call to cdb_mode_sense_geom(&dop, > &geomdata) to retrieve the info > related to cyl/head type stuff. On the older/working thumbdrive it > returns zeroes for all of the values > that get used by the code. The newer/failing drive generates a > "stall" on the USB bus, which it never > recovers from. The cdb_mode_sense_geom() function is sending a SCSI > CDB Mode Sense (CMD=0x5A) to the device. > > As a hack of a test, I removed the call to cdb_mode_sense_geom() and > filled the buffer it should have returned > with zeroes and the failing thumbdrive now boots. > > I have some searching I need to do to find out... > 1) Is there a SCSI command to determine what protocols are supported? > 2) Is there another SCSI command that might return similar required > data? > > Has anyone out there experienced similar booting difficulties? > Or does anyone have any recommendations on what approach I should > take? > > thanks, > Dave > > ___ > SeaBIOS mailing list > SeaBIOS@seabios.org > http://www.seabios.org/mailman/listinfo/seabios > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] Booting from USB thumbdrives, older drives boot, newer drives don't.
I'm a new subscriber to seabios.org so feel free to straighten me out if needed. I've been debugging an problem we've been seeing with not being able to boot (Ubuntu specifically) off of a variety of "newer" USB thumb drives. I've been specifically looking at an older/newer pair of Sandisk Cruzer 4GB drives. I've been adding dprintf's to narrow down the problem to the blockcmd.c file. The function scsi_init_drive() queries the USB device to determine stuff like vendor/device/size/etc. Near the end of the function is a call to cdb_mode_sense_geom(&dop, &geomdata) to retrieve the info related to cyl/head type stuff. On the older/working thumbdrive it returns zeroes for all of the values that get used by the code. The newer/failing drive generates a "stall" on the USB bus, which it never recovers from. The cdb_mode_sense_geom() function is sending a SCSI CDB Mode Sense (CMD=0x5A) to the device. As a hack of a test, I removed the call to cdb_mode_sense_geom() and filled the buffer it should have returned with zeroes and the failing thumbdrive now boots. I have some searching I need to do to find out... 1) Is there a SCSI command to determine what protocols are supported? 2) Is there another SCSI command that might return similar required data? Has anyone out there experienced similar booting difficulties? Or does anyone have any recommendations on what approach I should take? thanks, Dave ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios