Message ID | CALdTtnt1W=grqAEvNwwD-tq6v04EQSSmH=dbXkByRpVh7cx4Eg@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Wed, Mar 29, 2017 at 06:58:58PM -0600, dann frazier wrote: > This is a port of the LIBIO infrastructure being prepared upstream to > handle the low-pin-count (LPC) controller support needed for HiSilicon > server SoCs, and which is also evolving into a generic PIO mapping > system for the kernel. While the direction has upstream approval and > some patches are acked, it is still evolving. I've therefore only > enabled CONFIG_LIBIO on arm64, and also added a patch to revert to the > existing PCI PIO mapping code when CONFIG_LIBIO is disabled. > > Tested on a HiSilicon D05 board. Regression tested on ppc64el by Manoj > Iyer (for the OF change). Also regression tested on a Cavium ThunderX > system. Built tested on all Ubuntu archs in ppa:dannf/test. > > Addresses: https://bugs.launchpad.net/bugs/1677319 > > The following changes since commit 4bd7900ebaac4fd554f8b062954175a1de04686f: > > UBUNTU: SAUCE: (no-up) net/mlx5: Avoid dereferencing uninitialized > pointer (2017-03-29 06:19:51 -0600) > > are available in the git repository at: > > git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc > > for you to fetch changes up to 4354f8bbb202ae8374664b907d4a432e48cc057f: > > UBUNTU: SAUCE: PCI: Restore codepath for !CONFIG_LIBIO (2017-03-29 > 15:33:44 -0600) Some of this isn't exactly pretty. But it does look to keep nearly all the changes in behavior isolated to arm64. Additionally, a good deal of it is isolated to device-specific code. I think the OF changes look safe, and testing seems to bear that out. Given that we're very late in the zesty development cycle, the major concern I have is the extent of regression testing on other arm64 platforms, with regard to the libio/pci changes in particular. I think it would be better to try and get some broader testing, then SRU these a bit later. One curious thing I encountered while looking through the patches that I hadn't noticed previously: +static acpi_status +acpi_build_libiores_template(struct acpi_device *adev, + struct acpi_buffer *buffer) +{ + acpi_handle handle = adev->handle; + struct acpi_resource *resource; + acpi_status status; + int res_cnt = 0; + + status = acpi_walk_resources(handle, METHOD_NAME__CRS, + acpi_count_libiores, &res_cnt); + if (ACPI_FAILURE(status) || !res_cnt) { + dev_err(&adev->dev, "can't evaluate _CRS: %d\n", status); + return -EINVAL; + } + + buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1) + 1; + buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL); I don't understand allocating the buffer one byte less than buffer->length. Can you confirm that this isn't a bug? Thanks, Seth
On Thu, Mar 30, 2017 at 10:04 AM, Seth Forshee <seth.forshee@canonical.com> wrote: > On Wed, Mar 29, 2017 at 06:58:58PM -0600, dann frazier wrote: >> This is a port of the LIBIO infrastructure being prepared upstream to >> handle the low-pin-count (LPC) controller support needed for HiSilicon >> server SoCs, and which is also evolving into a generic PIO mapping >> system for the kernel. While the direction has upstream approval and >> some patches are acked, it is still evolving. I've therefore only >> enabled CONFIG_LIBIO on arm64, and also added a patch to revert to the >> existing PCI PIO mapping code when CONFIG_LIBIO is disabled. >> >> Tested on a HiSilicon D05 board. Regression tested on ppc64el by Manoj >> Iyer (for the OF change). Also regression tested on a Cavium ThunderX >> system. Built tested on all Ubuntu archs in ppa:dannf/test. >> >> Addresses: https://bugs.launchpad.net/bugs/1677319 >> >> The following changes since commit 4bd7900ebaac4fd554f8b062954175a1de04686f: >> >> UBUNTU: SAUCE: (no-up) net/mlx5: Avoid dereferencing uninitialized >> pointer (2017-03-29 06:19:51 -0600) >> >> are available in the git repository at: >> >> git://git.launchpad.net/~dannf/ubuntu/+source/linux/+git/linux lpc >> >> for you to fetch changes up to 4354f8bbb202ae8374664b907d4a432e48cc057f: >> >> UBUNTU: SAUCE: PCI: Restore codepath for !CONFIG_LIBIO (2017-03-29 >> 15:33:44 -0600) > > Some of this isn't exactly pretty. I don't disagree. > But it does look to keep nearly all > the changes in behavior isolated to arm64. Additionally, a good deal of > it is isolated to device-specific code. I think the OF changes look > safe, and testing seems to bear that out. Yeah - tested on Power, and 2 arm64 systems in device-tree mode that have PCIe (Cavium ThunderX CRB, HP X-Gene m400). > Given that we're very late in the zesty development cycle, the major > concern I have is the extent of regression testing on other arm64 > platforms, with regard to the libio/pci changes in particular. I think > it would be better to try and get some broader testing, then SRU these a > bit later. Also tested on a QDF2400, which is an arm64 server that boots in ACPI mode. > One curious thing I encountered while looking through the patches that I > hadn't noticed previously: > > +static acpi_status > +acpi_build_libiores_template(struct acpi_device *adev, > + struct acpi_buffer *buffer) > +{ > + acpi_handle handle = adev->handle; > + struct acpi_resource *resource; > + acpi_status status; > + int res_cnt = 0; > + > + status = acpi_walk_resources(handle, METHOD_NAME__CRS, > + acpi_count_libiores, &res_cnt); > + if (ACPI_FAILURE(status) || !res_cnt) { > + dev_err(&adev->dev, "can't evaluate _CRS: %d\n", status); > + return -EINVAL; > + } > + > + buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1) + 1; > + buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL); > > I don't understand allocating the buffer one byte less than > buffer->length. Can you confirm that this isn't a bug? Will do. Thanks for the (what is this now, 5th?) review! :) -dann
Applied to master-next on Zesty.