coreboot-gerrit February 2018

coreboot-gerrit@coreboot.org

1 participants
1573 discussions

Change in coreboot[master]: soc/intel/skylake: Add IOMMU support
by Nico Huber (Code Review) 10 Feb '18

10 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23682 ) Change subject: soc/intel/skylake: Add IOMMU support ...................................................................... Patch Set 1: (3 comments) I don't see any changes to the FSP UPDs. I guess you didn't test (or verify otherwise) which part of the configuration is done by the blob? I think we should start with a minimal setup (i.e. only setup BARs) and see what the blob configures. https://review.coreboot.org/#/c/23682/1/src/soc/intel/skylake/bootblock/pch… File src/soc/intel/skylake/bootblock/pch.c: PS1: I don't think the boot block is the right place for this. https://review.coreboot.org/#/c/23682/1/src/soc/intel/skylake/bootblock/pch… PS1, Line 64: /* Assign unique bus/dev/fn for I/O APIC and HPET */ Actually I've so far only read that you can assign unique numbers, but never why or for what it would be required. Do you know why? https://review.coreboot.org/#/c/23682/1/src/soc/intel/skylake/iommu.c File src/soc/intel/skylake/iommu.c: https://review.coreboot.org/#/c/23682/1/src/soc/intel/skylake/iommu.c@40 PS1, Line 40: /* lock policies */ : write32((void *)(IOMMU_BASE_ADDRESS1 + 0xff0), 0x80000000); : : const struct device *const azalia = dev_find_slot(0, PCH_DEVFN_HDA); : if (azalia && azalia->enabled) { : write32((void *)(IOMMU_BASE_ADDRESS2 + 0xff0), 0x20000000); : write32((void *)(IOMMU_BASE_ADDRESS2 + 0xff0), 0xa0000000); : } else { : write32((void *)(IOMMU_BASE_ADDRESS2 + 0xff0), 0x80000000); : } I suppose this is just copy-pasta? Or is there any clue that Skylake works like this too? -- To view, visit https://review.coreboot.org/23682 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifadaa11340406d1da0f98813589d20118744cc6f Gerrit-Change-Number: 23682 Gerrit-PatchSet: 1 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Feb 2018 18:03:16 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: soc/intel/skylake: Generate ACPI DMAR table
by build bot (Jenkins) (Code Review) 10 Feb '18

10 Feb '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21588 ) Change subject: soc/intel/skylake: Generate ACPI DMAR table ...................................................................... Patch Set 2: Verified-1 Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/67320/ : UNSTABLE https://qa.coreboot.org/job/coreboot-checkpatch/21866/ : SUCCESS -- To view, visit https://review.coreboot.org/21588 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8176401dd19aee7ad09a8a145b7a3801fe5b2ae1 Gerrit-Change-Number: 21588 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Feb 2018 17:19:57 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: [WIP] soc/intel/skylake: Hook up libgfxinit
by build bot (Jenkins) (Code Review) 10 Feb '18

10 Feb '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20113 ) Change subject: [WIP] soc/intel/skylake: Hook up libgfxinit ...................................................................... Patch Set 5: Verified-1 Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/67319/ : UNSTABLE https://qa.coreboot.org/job/coreboot-checkpatch/21865/ : SUCCESS -- To view, visit https://review.coreboot.org/20113 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I79fbfed7732cf02d054530ea7ec0d34de32836ba Gerrit-Change-Number: 20113 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Feb 2018 17:17:34 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: [TEST] Fix CAR for P4 CPU's
by build bot (Jenkins) (Code Review) 10 Feb '18

10 Feb '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23690 ) Change subject: [TEST] Fix CAR for P4 CPU's ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-gerrit/67318/ : SUCCESS https://qa.coreboot.org/job/coreboot-checkpatch/21864/ : SUCCESS -- To view, visit https://review.coreboot.org/23690 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00cf86259d5c5c6e46e41a516cf585bbc11a26a2 Gerrit-Change-Number: 23690 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Feb 2018 15:22:50 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: [TEST] Fix CAR for P4 CPU's
by Arthur Heymans (Code Review) 10 Feb '18

10 Feb '18

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/23690 Change subject: [TEST] Fix CAR for P4 CPU's ...................................................................... [TEST] Fix CAR for P4 CPU's when reading cpuid(4) it looks like the wrong ecx argument is provided (4 instead of 0), despite what to code does. This leads to a division by 0 later which has the code do wrong stuff. Change-Id: I00cf86259d5c5c6e46e41a516cf585bbc11a26a2 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/car/cache_as_ram_ht.inc 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/23690/1 diff --git a/src/cpu/intel/car/cache_as_ram_ht.inc b/src/cpu/intel/car/cache_as_ram_ht.inc index e716caf..64e6083 100644 --- a/src/cpu/intel/car/cache_as_ram_ht.inc +++ b/src/cpu/intel/car/cache_as_ram_ht.inc @@ -136,6 +136,7 @@ movb $1, %bl cmpl $4, %eax jb cores_counted + movl $0, %ecx movl $4, %eax movl $0, %ecx cpuid -- To view, visit https://review.coreboot.org/23690 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I00cf86259d5c5c6e46e41a516cf585bbc11a26a2 Gerrit-Change-Number: 23690 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

Change in coreboot[master]: libpayload/drivers/usb: Fix leaks
by Nico Huber (Code Review) 10 Feb '18

10 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23689 ) Change subject: libpayload/drivers/usb: Fix leaks ...................................................................... Patch Set 1: (3 comments) https://review.coreboot.org/#/c/23689/1/payloads/libpayload/drivers/usb/usb… File payloads/libpayload/drivers/usb/usb.c: https://review.coreboot.org/#/c/23689/1/payloads/libpayload/drivers/usb/usb… PS1, Line 673: if (dev->descriptor) No need to check for NULL, free(NULL); is valid. https://review.coreboot.org/#/c/23689/1/payloads/libpayload/drivers/usb/usb… PS1, Line 675: 0 What's wrong with NULL? https://review.coreboot.org/#/c/23689/1/payloads/libpayload/drivers/usb/usb… PS1, Line 673: if (dev->descriptor) : free(dev->descriptor); : dev->descriptor = 0; : if (dev->configuration) : free(dev->configuration); : dev->configuration = 0; IMO, this should be done after the call to ->destroy(). You shouldn't have to repeat it in every implementation... -- To view, visit https://review.coreboot.org/23689 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib2d80dd4590aa0dacdf2da3b614c6505c931d0be Gerrit-Change-Number: 23689 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Feb 2018 14:00:39 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: libpayload/drivers/usb: Fix broken retry counter
by Nico Huber (Code Review) 10 Feb '18

10 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23688 ) Change subject: libpayload/drivers/usb: Fix broken retry counter ...................................................................... Patch Set 1: Code-Review+2 (2 comments) https://review.coreboot.org/#/c/23688/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23688/1//COMMIT_MSG@7 PS1, Line 7: libpayload/drivers/usb maybe just "libpayload/usb:" https://review.coreboot.org/#/c/23688/1/payloads/libpayload/drivers/usb/usb… File payloads/libpayload/drivers/usb/usb.c: https://review.coreboot.org/#/c/23688/1/payloads/libpayload/drivers/usb/usb… PS1, Line 180: return len; Simpler: if (ret == len) break; udelay(10); -- To view, visit https://review.coreboot.org/23688 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c881c08d94636a43223338e46c876b5f3e27d47 Gerrit-Change-Number: 23688 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Shawn Nematbakhsh <shawnn(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Feb 2018 13:52:24 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: libpayload/drivers/usb/uhci: Fix infinite loop
by Nico Huber (Code Review) 10 Feb '18

10 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23687 ) Change subject: libpayload/drivers/usb/uhci: Fix infinite loop ...................................................................... Patch Set 1: Not your fault, but I don't like this at all. The original assumption really was that we always enumerate present devices and keep the pro- gram state in sync with the hardware (i.e. an unused device has an address assigned the state of the USB stack should know that). If we assign addresses to devices on the bus and then ignore the device plus remove that information that the address is assigned, we may end up with multiple devices on the bus with the same address. I guess, we didn't observe this yet, because a) xHCI doesn't care (the controller assigns the addresses) and b) all other controllers have 127 addresses available before it wraps around. Ideas how to cope with it: 1) Make sure (still present) slave devices are reset when usb_detach_device() is called, store a hint that we don't want to reenumerate the port unless it was detached in between. 2) Revert the offending changes that added usb_detach_device() calls for present devices without intention to re-enumerate the port. I'd really prefer 2). Along with going through the hub drivers to make sure they re-enumerate when necessary. -- To view, visit https://review.coreboot.org/23687 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic652311e995e7addd807d2dda8e1c8f385a0d45c Gerrit-Change-Number: 23687 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Feb 2018 13:48:40 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in coreboot[master]: libpayload/drivers/usb/uhci: Fix regression
by Nico Huber (Code Review) 10 Feb '18

10 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23686 ) Change subject: libpayload/drivers/usb/uhci: Fix regression ...................................................................... Patch Set 1: Code-Review+1 (4 comments) https://review.coreboot.org/#/c/23686/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23686/1//COMMIT_MSG@7 PS1, Line 7: libpayload/drivers/usb/uhci: I guess "libpayload/uhci:" would suffice. https://review.coreboot.org/#/c/23686/1//COMMIT_MSG@7 PS1, Line 7: Fix regression Please mention what it's actually about, e.g. "Return transferred byte count". https://review.coreboot.org/#/c/23686/1//COMMIT_MSG@10 PS1, Line 10: introduced a regression on I would actually call it what it is, i.e. "deliberately broke". It even says so in the commit message; how the hell could that get merged? https://review.coreboot.org/#/c/23686/1/payloads/libpayload/drivers/usb/uhc… File payloads/libpayload/drivers/usb/uhci.c: https://review.coreboot.org/#/c/23686/1/payloads/libpayload/drivers/usb/uhc… PS1, Line 458: return 0; And here? -- To view, visit https://review.coreboot.org/23686 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I823521e24f6f9e04c86d36fda751c449cd884ce3 Gerrit-Change-Number: 23686 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Feb 2018 13:22:27 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: libpayload/drivers/usb: Fix leaks
by build bot (Jenkins) (Code Review) 10 Feb '18

10 Feb '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23689 ) Change subject: libpayload/drivers/usb: Fix leaks ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-gerrit/67317/ : SUCCESS https://qa.coreboot.org/job/coreboot-checkpatch/21863/ : SUCCESS -- To view, visit https://review.coreboot.org/23689 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib2d80dd4590aa0dacdf2da3b614c6505c931d0be Gerrit-Change-Number: 23689 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Feb 2018 11:40:50 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2018