OpenBIOS

openbios@openbios.org

2 participants
4283 discussions

Re: [PATCH 4/7] drivers/usb.c: rework descriptor read logic in get_descriptor()
by Mark Cave-Ayland 25 Apr '24

25 Apr '24

On 24/04/2024 15:48, Philippe Mathieu-Daudé wrote: > Hi Mark, > > On 22/4/24 09:54, Mark Cave-Ayland wrote: >> When compiling with gcc 12 drivers/usb.c generates the following error: >> >> /root/drivers/usb.c: In function 'get_descriptor': >> /root/drivers/usb.c:200:23: error: array subscript 'device_descriptor_t[0]' is >> partly outside array bounds of 'u8[8]' {aka 'unsigned char[8]'} [-Werror=array-bounds] >> 200 | if (dd->bMaxPacketSize0 != 0) >> | ^~ >> /root/drivers/usb.c:181:12: note: object 'buf' of size 8 >> 181 | u8 buf[8]; >> | ^~~ >> cc1: all warnings being treated as errors >> make[1]: *** [rules.mak:229: target/drivers/usb.o] Error 1 >> make[1]: Leaving directory '/root/obj-ppc' >> >> The compiler is complaining that a device_descriptor_t pointer cast to the 8 byte >> buffer >> means that a pointer dereference to some members of the device_descriptor_t could go >> beyond the end of the 8 byte buffer. Whilst this sounds as if it should be allowed, >> some >> searching suggests that the behaviour of such a cast is undefined. >> >> Rework get_descriptor() so that an entire device_descriptor_t is used to store the >> first >> 8 bytes of the incoming descriptor, and update the buf references to use the >> equivalent >> device_descriptor_t fields instead. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> >> --- >> drivers/usb.c | 17 ++++++++--------- >> 1 file changed, 8 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/usb.c b/drivers/usb.c >> index 88b7580..d0e1ae5 100644 >> --- a/drivers/usb.c >> +++ b/drivers/usb.c >> @@ -178,7 +178,7 @@ u8 * >> get_descriptor (usbdev_t *dev, unsigned char bmRequestType, int descType, >> int descIdx, int langID) >> { >> - u8 buf[8]; >> + device_descriptor_t dd; >> u8 *result; >> dev_req_t dr; >> int size; >> @@ -189,24 +189,23 @@ get_descriptor (usbdev_t *dev, unsigned char bmRequestType, >> int descType, >> dr.wValue = __cpu_to_le16((descType << 8) | descIdx); >> dr.wIndex = __cpu_to_le16(langID); >> dr.wLength = __cpu_to_le16(8); >> - if (dev->controller->control (dev, IN, sizeof (dr), &dr, 8, buf)) { >> + if (dev->controller->control (dev, IN, sizeof (dr), &dr, 8, (u8 *)&dd)) { > > Still using dalen=8 seems odd. You only fill the first 8 bytes of dd, > leaving the rest uninitialized. Looking at which fields are used, it > is OK, but hard to review and possibly bugprone if this function were > modified in the future. > > Why not use sizeof (dd) instead? That still fits into a page, so > performance overhead is unlikely. > > My 2 cents, otherwise LGTM. Thanks! I did consider whether this would matter, but given that only the first 8 bytes were accessed it didn't seem to matter where the extra junk at the end of device_descriptor_t came from, so I kept it simple. The corresponding file upstream is very different from what is in OpenBIOS, so possibly this could be made clearer with an update, but that requires quite a bit more work :/ >> usb_debug ("getting descriptor size (type %x) failed\n", >> descType); >> } >> if (descType == 1) { >> - device_descriptor_t *dd = (device_descriptor_t *) buf; >> - usb_debug ("maxPacketSize0: %x\n", dd->bMaxPacketSize0); >> - if (dd->bMaxPacketSize0 != 0) >> - dev->endpoints[0].maxpacketsize = dd->bMaxPacketSize0; >> + usb_debug ("maxPacketSize0: %x\n", dd.bMaxPacketSize0); >> + if (dd.bMaxPacketSize0 != 0) >> + dev->endpoints[0].maxpacketsize = dd.bMaxPacketSize0; >> } >> /* special case for configuration descriptors: they carry all their >> subsequent descriptors with them, and keep the entire size at a >> different location */ >> - size = buf[0]; >> - if (buf[1] == 2) { >> - int realsize = __le16_to_cpu(((unsigned short *) (buf + 2))[0]); >> + size = dd.bLength; >> + if (dd.bDescriptorType == 2) { >> + int realsize = __le16_to_cpu(dd.bcdUSB); >> size = realsize; >> } >> result = malloc (size); ATB, Mark.

1 0

[PATCH 0/7] Build updates and improvements
by Mark Cave-Ayland 24 Apr '24

24 Apr '24

This series is based upon Amadeusz Sławiński's recent series at https://mail.coreboot.org/hyperkitty/list/openbios@openbios.org/thread/OLS2… and updates the code to fix build issues on gcc 12 (as used by Debian stable), whilst also correcting the compiler flags so that the binaries are generated correctly. Finally the github workflows are updated to use the Debian cross-compilers, and the GitHub release action changed to use an actively maintained fork. [MCA: switching to the Debian cross-compilers substantially lowers the barrier to entry for people setting up their own compile environments and also brings OpenBIOS in line with QEMU's own CI cross-compiler containers]. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> Mark Cave-Ayland (7): .github/workflows: update to latest action releases arch/ppc/qemu/init.c: work around GCC bug for ppc64_patch_handlers() packages/pc-parts.c: fix bug in extended partition logic drivers/usb.c: rework descriptor read logic in get_descriptor() switch-arch: disable generation of position independent code (-fno-pic) docker: switch from kernel crosstools compilers to Debian cross-compilers .github/workflows: migrate release action to crowbarmaster/GH-Automatic-Releases .github/workflows/build-openbios-builder.yml | 8 ++++---- .github/workflows/main.yml | 8 ++++---- .github/workflows/release.yml | 8 ++++---- arch/ppc/qemu/init.c | 12 ++++++++++++ config/scripts/switch-arch | 10 +++++----- docker/Dockerfile.builder | 18 ++---------------- drivers/usb.c | 17 ++++++++--------- packages/pc-parts.c | 7 +++++-- 8 files changed, 44 insertions(+), 44 deletions(-) -- 2.39.2

2 11

[PATCH] kernel: fix spelling of endianness
by Mark Cave-Ayland 24 Apr '24

24 Apr '24

This is based upon a similar patch currently being carried in the QEMU Debian package sources. Reported-by: Michael Tokarev <mjt(a)tls.msk.ru> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> --- kernel/bootstrap.c | 4 ++-- kernel/dict.c | 2 +- kernel/include/dict.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/bootstrap.c b/kernel/bootstrap.c index 9bc94bb..8ad3efe 100644 --- a/kernel/bootstrap.c +++ b/kernel/bootstrap.c @@ -171,9 +171,9 @@ static void write_dictionary(const char *filename) .version = 2, .cellsize = sizeof(ucell), #ifdef CONFIG_BIG_ENDIAN - .endianess = -1, + .endianness = -1, #else - .endianess = 0, + .endianness = 0, #endif .checksum = 0, .compression = 0, diff --git a/kernel/dict.c b/kernel/dict.c index 0986cb1..51afc67 100644 --- a/kernel/dict.c +++ b/kernel/dict.c @@ -252,7 +252,7 @@ void dump_header(dictionary_header_t *header) printk("OpenBIOS dictionary:\n"); printk(" version: %d\n", header->version); printk(" cellsize: %d\n", header->cellsize); - printk(" endianess: %s\n", header->endianess?"big":"little"); + printk(" endianness: %s\n", header->endianness?"big":"little"); printk(" compression: %s\n", header->compression?"yes":"no"); printk(" relocation: %s\n", header->relocation?"yes":"no"); printk(" checksum: %08x\n", target_long(header->checksum)); diff --git a/kernel/include/dict.h b/kernel/include/dict.h index 749fd6f..1e735fa 100644 --- a/kernel/include/dict.h +++ b/kernel/include/dict.h @@ -27,7 +27,7 @@ typedef struct dictionary_header { char signature[8]; u8 version; u8 cellsize; - u8 endianess; + u8 endianness; u8 compression; u8 relocation; u8 reserved[3]; -- 2.39.2

1 0

[PATCH 0/7] Fix build issues
by Amadeusz Sławiński 24 Apr '24

24 Apr '24

Set of patches for few problems found when trying to build project. Also enable some additional checks and also fix issues found. Amadeusz Sławiński (7): adb_kbd.c: Fix function key definitions Change malloc to take size_t as argument Label fallthroughs in switch()es Makefile.target: Enable few additional warnings Disable stack protector on all architectures bootstrap: Check result of fread Tell linker that stack is not executable Makefile.target | 2 ++ arch/amd64/lib.c | 2 +- arch/ppc/build.xml | 8 ++++---- arch/ppc/ofmem.c | 2 +- arch/ppc/qemu/ofmem.c | 2 +- arch/sparc32/lib.c | 2 +- arch/sparc64/lib.c | 2 +- arch/x86/lib.c | 2 +- config/scripts/switch-arch | 14 +++++++------- drivers/adb_bus.c | 2 +- drivers/adb_kbd.c | 2 +- include/libc/stdlib.h | 2 +- kernel/bootstrap.c | 6 +++++- libc/vsprintf.c | 1 + 14 files changed, 28 insertions(+), 21 deletions(-) -- 2.43.2

2 9

GRUB causes Unhandled Exception in firmware for sparc64 build
by Glenn Washburn 01 Aug '23

01 Aug '23

Hi all, I've been running into an issue with a GRUB test where it is causing an Unhandled Exception from within the openbios sparc64 firmware. I've created an issue[1] on github mainly to store the reproducing ISO. I'm not sure what the root issue is, but it seems to have something to do with having the firmware open the floppy drive. This happens on a build from master, the latest release build, and an old build packaged with Debian. None of the other GRUB tests trigger this bug. Any ideas what's going on? Glenn [1] https://github.com/openbios/openbios/issues/14

1 0

[PATCH] kernel: Fix compiler format string warnings when some debugging options are enabled
by Glenn Washburn 29 Jul '23

29 Jul '23

Signed-off-by: Glenn Washburn <development(a)efficientek.com> --- This compile-time errors from GCC 10.1.0 when the config options CONFIG_DEBUG_BOOT, CONFIG_DEBUG_DICTIONARY, and CONFIG_DEBUG_INTERPRETER are set to true in the ppc64_config.xml. I suspect these are issues for 32-bit PPC as well, but haven't checked. Glenn --- kernel/bootstrap.c | 8 ++++---- kernel/dict.c | 4 ++-- kernel/forth.c | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/kernel/bootstrap.c b/kernel/bootstrap.c index b7658ab..6834597 100644 --- a/kernel/bootstrap.c +++ b/kernel/bootstrap.c @@ -133,9 +133,9 @@ static void relocation_table(unsigned char * dict_one, unsigned char *dict_two, } #ifdef CONFIG_DEBUG_DICTIONARY - printk("dict1 %lx dict2 %lx dict %lx\n",dict_one, dict_two, dict); + printk("dict1 %p dict2 %p dict %p\n",dict_one, dict_two, dict); for (i=0; i< relocation_length ; i++) - printk("reloc %d %lx\n",i+1, reloc_table[i]); + printk("reloc %d "FMT_ucellx"\n",i+1, reloc_table[i]); #endif relocation_address=reloc_table; } @@ -819,7 +819,7 @@ static int build_dictionary(void) dicthead = 0; #ifdef CONFIG_DEBUG_DICTIONARY - printk("building dictionary, %d primitives.\nbuilt words:", + printk("building dictionary, %lu primitives.\nbuilt words:", sizeof(wordnames) / sizeof(void *)); #endif @@ -1248,7 +1248,7 @@ int main(int argc, char *argv[]) printf("dict1: %p\n",bootstrapdict[0]); printf("dict2: %p\n",bootstrapdict[1]); printf("trampoline: %p\n",trampoline); - printf("size=%d, trampoline_size=%d\n",MEMORY_SIZE + (2 * + printf("size=%ld, trampoline_size=%ld\n",MEMORY_SIZE + (2 * DICTIONARY_SIZE) + TRAMPOLINE_SIZE, TRAMPOLINE_SIZE); #endif diff --git a/kernel/dict.c b/kernel/dict.c index 0986cb1..e449c5b 100644 --- a/kernel/dict.c +++ b/kernel/dict.c @@ -293,8 +293,8 @@ ucell load_dictionary(const char *data, ucell len) reloc_table=(ucell *)(data+dicthead); #ifdef CONFIG_DEBUG_DICTIONARY - printk("\nmoving dictionary (%x bytes) to %x\n", - (ucell)dicthead, (ucell)dict); + printk("\nmoving dictionary (%x bytes) to "FMT_ucellx"\n", + (ucell)dicthead, pointer2cell(dict)); printk("\ndynamic relocation..."); #endif diff --git a/kernel/forth.c b/kernel/forth.c index 61dd70d..ca3fda3 100644 --- a/kernel/forth.c +++ b/kernel/forth.c @@ -111,7 +111,7 @@ static void docol(void) PUSHR(PC); PC = read_ucell(cell2pointer(PC)); - dbg_interp_printk("docol: %s\n", cell2pointer( lfa2nfa(PC - sizeof(cell)) )); + dbg_interp_printk("docol: %s\n", (char *)cell2pointer( lfa2nfa(PC - sizeof(cell)) )); } static void semis(void) @@ -719,7 +719,7 @@ static void docol_dbg(void) debug_xt_item = debug_xt_item->next; } - dbg_interp_printk("docol_dbg: %s\n", cell2pointer(lfa2nfa(PC - sizeof(cell)))); + dbg_interp_printk("docol_dbg: %s\n", (char *)cell2pointer(lfa2nfa(PC - sizeof(cell)))); } static void semis_dbg(void) -- 2.30.2

2 4

[PATCH 0/3] Simplify pci.c by moving some parts to forth
by BALATON Zoltan 11 May '23

11 May '23

This is untested and probably should be marked RFC as it's mainly to show what I was suggesting as an alternative fix for the decode-unit problem discovered by Mark. This could be taken further to adopt the decode-unit and encode-unit words from openfirmware to reduce clutter in pci.c even more which I may attempt if this direction is accepted. My config words patch will also need to be rebased but let this part settle fitst and I can submit the rest as a follow up or a v2 for this series later. Regards, BALATON Zoltan (3): pci: Use forth to add trivial open/close methods pci: Add is-pci-bus word to install some methods from forth pci: Fix calling decode-unit without active-package drivers/pci.c | 78 +++++--------------------------------------------- drivers/pci.fs | 10 +++++++ 2 files changed, 17 insertions(+), 71 deletions(-) -- 2.30.8

3 16

[PATCH 0/6] Fixes for Forth PCI config access words and implementation
by Mark Cave-Ayland 14 Mar '23

14 Mar '23

This series provides a set of fixes required to allow Forth PCI config access words to work correctly on both 32-bit and 64-bit platforms, along with Zoltan's original v4 patch. There are 2 main fixes included: - Implement and convert PCI encode-unit/decode-unit to static methods - Fix my-address and my-unit on 64-bit platforms With this series applied it is possible to execute the Forth PCI config words as documented in various drivers as below: PPC (32-bit): 0 > showstack ok 0 > " /pci/QEMU,VGA" open-dev to my-self ok 0 > my-space " config-w@" $call-parent u. 1234 ok 0 > my-space ok 800 < 1 > my-address ok 800 0 0 < 3 > SPARC64 (64-bit, including across PCI bridge): 0 > showstack ok 0 > " /pci/pci@1,1/QEMU,VGA" open-dev to my-self ok 0 > my-space " config-w@" $call-parent u. 1234 ok 0 > my-space ok 11000 < 1 > my-address ok 11000 0 0 < 3 > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> BALATON Zoltan (1): drivers/pci: Implement config access words Mark Cave-Ayland (5): libopenbios/clib.fs: add new is-2arg-cfunc for calling into C libopenbios: introduce BIND_NODE_STATIC_METHOD and supporting functions drivers/pci.c: convert encode-unit and decode-unit words to static methods package.fs: fix my-address on 64-bit systems package.fs: fix my-unit on 64-bit systems drivers/pci.c | 115 +++++++++++++++++++++++++++++++-- forth/device/package.fs | 8 +-- include/libopenbios/bindings.h | 7 ++ libopenbios/bindings.c | 26 ++++++++ libopenbios/clib.fs | 10 +++ 5 files changed, 155 insertions(+), 11 deletions(-) -- 2.30.2

2 12

[PATCH] usbhid: Implement get-key-map method
by BALATON Zoltan 07 Mar '23

07 Mar '23

Mac OS X calls it during boot. This gets rid of an error saying "call-method slw_update_keymap failed with error ffffffdf" but otherwise does not seem to matter. It's probably used for getting the pressed keys that can change some boot actions like enable verbose mode with Cmd-V. Signed-off-by: BALATON Zoltan <balaton(a)eik.bme.hu> --- drivers/usbhid.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/usbhid.c b/drivers/usbhid.c index b8e1548..d3ad4ef 100644 --- a/drivers/usbhid.c +++ b/drivers/usbhid.c @@ -52,12 +52,20 @@ keyboard_close(int *idx) { } +static void +keyboard_get_key_map(void) +{ + static char keytable[32] = { 0 }; + PUSH(pointer2cell(keytable)); +} + static void keyboard_read(void); NODE_METHODS( usb_kbd ) = { { "open", keyboard_open }, { "close", keyboard_close }, { "read", keyboard_read }, + { "get-key-map", keyboard_get_key_map }, }; #ifdef CONFIG_DEBUG_USB -- 2.30.6

2 5

[PATCH v4] drivers/pci: Implement config access words
by BALATON Zoltan 06 Mar '23

06 Mar '23

Signed-off-by: BALATON Zoltan <balaton(a)eik.bme.hu> --- v4: Added call parent word for bridge devices drivers/pci.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/drivers/pci.c b/drivers/pci.c index f30e427..084ebe9 100644 --- a/drivers/pci.c +++ b/drivers/pci.c @@ -424,6 +424,60 @@ ob_pci_bus_map_in(int *idx) PUSH(virt); } +static void +ob_pci_config_read8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint8_t val = pci_config_read8(addr, hi & 0xff); + PUSH(val); +} + +static void +ob_pci_config_write8(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write8(addr, hi & 0xff, val); +} + +static void +ob_pci_config_read16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint16_t val = pci_config_read16(addr, hi & 0xff); + PUSH(val); +} + +static void +ob_pci_config_write16(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write16(addr, hi & 0xff, val); +} + +static void +ob_pci_config_read32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + uint32_t val = pci_config_read32(addr, hi & 0xff); + PUSH(val); +} + +static void +ob_pci_config_write32(int *idx) +{ + cell hi = POP(); + pci_addr addr = PCI_ADDR(PCI_BUS(hi), PCI_DEV(hi), PCI_FN(hi)); + cell val = POP(); + pci_config_write32(addr, hi & 0xff, val); +} + static void ob_pci_dma_alloc(int *idx) { @@ -460,6 +514,12 @@ NODE_METHODS(ob_pci_bus_node) = { { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bus_map_in }, + { "config-b@", ob_pci_config_read8 }, + { "config-b!", ob_pci_config_write8 }, + { "config-w@", ob_pci_config_read16 }, + { "config-w!", ob_pci_config_write16 }, + { "config-l@", ob_pci_config_read32 }, + { "config-l!", ob_pci_config_write32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in }, @@ -476,12 +536,54 @@ ob_pci_bridge_map_in(int *idx) call_parent_method("pci-map-in"); } +static void +ob_pci_bridge_cread8(void) +{ + call_parent_method("config-b@"); +} + +static void +ob_pci_bridge_cwrite8(void) +{ + call_parent_method("config-b!"); +} + +static void +ob_pci_bridge_cread16(void) +{ + call_parent_method("config-w@"); +} + +static void +ob_pci_bridge_cwrite16(void) +{ + call_parent_method("config-w!"); +} + +static void +ob_pci_bridge_cread32(void) +{ + call_parent_method("config-l@"); +} + +static void +ob_pci_bridge_cwrite32(void) +{ + call_parent_method("config-l!"); +} + NODE_METHODS(ob_pci_bridge_node) = { { "open", ob_pci_open }, { "close", ob_pci_close }, { "decode-unit", ob_pci_decode_unit }, { "encode-unit", ob_pci_encode_unit }, { "pci-map-in", ob_pci_bridge_map_in }, + { "config-b@", ob_pci_bridge_cread8 }, + { "config-b!", ob_pci_bridge_cwrite8 }, + { "config-w@", ob_pci_bridge_cread16 }, + { "config-w!", ob_pci_bridge_cwrite16 }, + { "config-l@", ob_pci_bridge_cread32 }, + { "config-l!", ob_pci_bridge_cwrite32 }, { "dma-alloc", ob_pci_dma_alloc }, { "dma-free", ob_pci_dma_free }, { "dma-map-in", ob_pci_dma_map_in }, -- 2.30.6

2 3

Jump to page: