Skip to contentSkip to conversation

arm64: spin-table: handle unmapped cpu-release-addrs

PATCH New Mark Rutland 6 participants 12 messages Aug 13, 2014 – Aug 18, 2014 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): CAKv+Gu_UjRNhhiM0GPsKRdXRtmEnY6cbpY-JZ33RUMapbPYsbQ@mail.gmail.com

#1 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Hi Ard,

Quoted 11 lines

Gents,

With the 3.17 merge window not far from closing, is there anything we could still do this cycle to get $subject addressed?
As I have indicated before, the newly introduced TEXT_OFFSET fuzzing may break APM Mustang if booting from UEFI (i.e., for low values of rand()), and as this option is recommended for distribution kernels, it could make apt-get update'ing your kernel a frustrating experience. However, the fix for that issue triggers the issue addressed by $subject.

Quoted 6 lines

In my personal opinion, relaxing the requirements imposed on where your cpu-release-addr may live could remain a separate discussion. In the APM Mustang case, even when adhering to booting.txt by the letter (i.e., pu cpu-release-addr in a memreserved area in DRAM and load Image at a 2 meg boundary + TEXT_OFFSET), the kernel fails to bring up the secondaries if the cpu-release-addr is below the kernel.

For the moment I would be happy with the patch as it stands (using ioremap_cache).

Catalin, Will?

Mark.

#2 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 16 lines · Mark Rutland
Quoted 9 lines

With the 3.17 merge window not far from closing, is there anything we could still do this cycle to get $subject addressed?
As I have indicated before, the newly introduced TEXT_OFFSET fuzzing may break APM Mustang if booting from UEFI (i.e., for low values of rand()), and as this option is recommended for distribution kernels, it could make apt-get update'ing your kernel a frustrating experience. However, the fix for that issue triggers the issue addressed by $subject.

Quoted 6 lines

In my personal opinion, relaxing the requirements imposed on where your cpu-release-addr may live could remain a separate discussion. In the APM Mustang case, even when adhering to booting.txt by the letter (i.e., pu cpu-release-addr in a memreserved area in DRAM and load Image at a 2 meg boundary + TEXT_OFFSET), the kernel fails to bring up the secondaries if the cpu-release-addr is below the kernel.

Quoted 4 lines

For the moment I would be happy with the patch as it stands (using ioremap_cache).

Catalin, Will?

Using ioremap_cache() sounds fine to me but I have to dig the original patches as I've only been cc'ed in the middle of the thread (and that's just for patch 1/3).

Signature · 1 line
Catalin
#3 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 17 lines · Catalin Marinas
Quoted 16 lines · Mark Rutland
Quoted 15 lines

With the 3.17 merge window not far from closing, is there anything we could still do this cycle to get $subject addressed?
As I have indicated before, the newly introduced TEXT_OFFSET fuzzing may break APM Mustang if booting from UEFI (i.e., for low values of rand()), and as this option is recommended for distribution kernels, it could make apt-get update'ing your kernel a frustrating experience. However, the fix for that issue triggers the issue addressed by $subject.

In my personal opinion, relaxing the requirements imposed on where your cpu-release-addr may live could remain a separate discussion. In the APM Mustang case, even when adhering to booting.txt by the letter (i.e., pu cpu-release-addr in a memreserved area in DRAM and load Image at a 2 meg boundary + TEXT_OFFSET), the kernel fails to bring up the secondaries if the cpu-release-addr is below the kernel.

Quoted 9 lines
Quoted 4 lines

For the moment I would be happy with the patch as it stands (using ioremap_cache).

Catalin, Will?

Using ioremap_cache() sounds fine to me but I have to dig the original patches as I've only been cc'ed in the middle of the thread (and that's just for patch 1/3).

http://marc.info/?l=linux-arm-kernel&m=140671806332258&w=2
http://marc.info/?l=linux-arm-kernel&m=140671807532262&w=2
http://marc.info/?l=linux-arm-kernel&m=140671807732263&w=2

Patch 3/3 makes the EFI stub load Image at the lowest 2 meg boundary + TEXT_OFFSET instead of erroring out when base of DRAM + TEXT_OFFSET is occupied. This is necessary as the randomized TEXT_OFFSET could potentially conflict with reserved areas at the base of DRAM if a low value happens to be chosen.

Patch 1/3 fixes the resulting breakage on APM Mustang in bringing up the secondaries, as on that board in particular, the reserved area at the base of DRAM contains the holding pen, and loading Image higher up makes the mailbox inaccessible through the linear mapping hence the need for ioremap_cache()

Patch 2/3 is also a fix for a potential issue on UEFI boot, but it is unrelated to 1/3 and 3/3

Signature · 1 line
Ard.
#4 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 38 lines · Ard Biesheuvel
Quoted 17 lines · Catalin Marinas
Quoted 16 lines · Mark Rutland
Quoted 15 lines

With the 3.17 merge window not far from closing, is there anything we could still do this cycle to get $subject addressed?
As I have indicated before, the newly introduced TEXT_OFFSET fuzzing may break APM Mustang if booting from UEFI (i.e., for low values of rand()), and as this option is recommended for distribution kernels, it could make apt-get update'ing your kernel a frustrating experience. However, the fix for that issue triggers the issue addressed by $subject.

In my personal opinion, relaxing the requirements imposed on where your cpu-release-addr may live could remain a separate discussion. In the APM Mustang case, even when adhering to booting.txt by the letter (i.e., pu cpu-release-addr in a memreserved area in DRAM and load Image at a 2 meg boundary + TEXT_OFFSET), the kernel fails to bring up the secondaries if the cpu-release-addr is below the kernel.

Quoted 9 lines
Quoted 4 lines

For the moment I would be happy with the patch as it stands (using ioremap_cache).

Catalin, Will?

Using ioremap_cache() sounds fine to me but I have to dig the original patches as I've only been cc'ed in the middle of the thread (and that's just for patch 1/3).

http://marc.info/?l=linux-arm-kernel&m=140671806332258&w=2
http://marc.info/?l=linux-arm-kernel&m=140671807532262&w=2
http://marc.info/?l=linux-arm-kernel&m=140671807732263&w=2

Patch 3/3 makes the EFI stub load Image at the lowest 2 meg boundary + TEXT_OFFSET instead of erroring out when base of DRAM + TEXT_OFFSET is occupied. This is necessary as the randomized TEXT_OFFSET could potentially conflict with reserved areas at the base of DRAM if a low value happens to be chosen.

Quoted 8 lines

Patch 1/3 fixes the resulting breakage on APM Mustang in bringing up the secondaries, as on that board in particular, the reserved area at the base of DRAM contains the holding pen, and loading Image higher up makes the mailbox inaccessible through the linear mapping hence the need for ioremap_cache()

Patch 2/3 is also a fix for a potential issue on UEFI boot, but it is unrelated to 1/3 and 3/3

I was planning to take all of these for 3.18 as there's no regression here (the fuzzing is a new debug feature and defaults to `n'). Do you think these qualify as -rc1 material?

Will

#5 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 45 lines · Will Deacon
Quoted 44 lines · Ard Biesheuvel
Quoted 17 lines · Catalin Marinas
Quoted 16 lines · Mark Rutland
Quoted 15 lines

With the 3.17 merge window not far from closing, is there anything we could still do this cycle to get $subject addressed?
As I have indicated before, the newly introduced TEXT_OFFSET fuzzing may break APM Mustang if booting from UEFI (i.e., for low values of rand()), and as this option is recommended for distribution kernels, it could make apt-get update'ing your kernel a frustrating experience. However, the fix for that issue triggers the issue addressed by $subject.

In my personal opinion, relaxing the requirements imposed on where your cpu-release-addr may live could remain a separate discussion. In the APM Mustang case, even when adhering to booting.txt by the letter (i.e., pu cpu-release-addr in a memreserved area in DRAM and load Image at a 2 meg boundary + TEXT_OFFSET), the kernel fails to bring up the secondaries if the cpu-release-addr is below the kernel.

Quoted 9 lines
Quoted 4 lines

For the moment I would be happy with the patch as it stands (using ioremap_cache).

Catalin, Will?

Using ioremap_cache() sounds fine to me but I have to dig the original patches as I've only been cc'ed in the middle of the thread (and that's just for patch 1/3).

http://marc.info/?l=linux-arm-kernel&m=140671806332258&w=2
http://marc.info/?l=linux-arm-kernel&m=140671807532262&w=2
http://marc.info/?l=linux-arm-kernel&m=140671807732263&w=2

Patch 3/3 makes the EFI stub load Image at the lowest 2 meg boundary + TEXT_OFFSET instead of erroring out when base of DRAM + TEXT_OFFSET is occupied. This is necessary as the randomized TEXT_OFFSET could potentially conflict with reserved areas at the base of DRAM if a low value happens to be chosen.

Patch 1/3 fixes the resulting breakage on APM Mustang in bringing up the secondaries, as on that board in particular, the reserved area at the base of DRAM contains the holding pen, and loading Image higher up makes the mailbox inaccessible through the linear mapping hence the need for ioremap_cache()

Quoted 7 lines
Quoted 2 lines

Patch 2/3 is also a fix for a potential issue on UEFI boot, but it is unrelated to 1/3 and 3/3

I was planning to take all of these for 3.18 as there's no regression here (the fuzzing is a new debug feature and defaults to `n'). Do you think these qualify as -rc1 material?

Considering that TEXT_OFFSET fuzzing is recommended to be turned on for distro kernels, I would say this is definitely appropriate for 3.17

Signature · 1 line
Ard.
#6 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 17 lines · Ard Biesheuvel
Quoted 16 lines · Will Deacon
Quoted 14 lines · Ard Biesheuvel

Patch 3/3 makes the EFI stub load Image at the lowest 2 meg boundary + TEXT_OFFSET instead of erroring out when base of DRAM + TEXT_OFFSET is occupied. This is necessary as the randomized TEXT_OFFSET could potentially conflict with reserved areas at the base of DRAM if a low value happens to be chosen.

Patch 1/3 fixes the resulting breakage on APM Mustang in bringing up the secondaries, as on that board in particular, the reserved area at the base of DRAM contains the holding pen, and loading Image higher up makes the mailbox inaccessible through the linear mapping hence the need for ioremap_cache()

Patch 2/3 is also a fix for a potential issue on UEFI boot, but it is unrelated to 1/3 and 3/3

Quoted 8 lines
Quoted 4 lines

I was planning to take all of these for 3.18 as there's no regression here (the fuzzing is a new debug feature and defaults to `n'). Do you think these qualify as -rc1 material?

Considering that TEXT_OFFSET fuzzing is recommended to be turned on for distro kernels, I would say this is definitely appropriate for 3.17

Whilst I see that in the commit log, the same recommendation doesn't appear in the Kconfig text and I'm not sure that it's such a wise thing to say either. From a distribution's point of view, I think I'd want any kernel issues to be as reproducible as possible, and fuzzing the text offset seems to go against that.

Will

#7 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 27 lines · Will Deacon
Quoted 17 lines · Ard Biesheuvel
Quoted 16 lines · Will Deacon
Quoted 14 lines · Ard Biesheuvel

Patch 3/3 makes the EFI stub load Image at the lowest 2 meg boundary + TEXT_OFFSET instead of erroring out when base of DRAM + TEXT_OFFSET is occupied. This is necessary as the randomized TEXT_OFFSET could potentially conflict with reserved areas at the base of DRAM if a low value happens to be chosen.

Patch 1/3 fixes the resulting breakage on APM Mustang in bringing up the secondaries, as on that board in particular, the reserved area at the base of DRAM contains the holding pen, and loading Image higher up makes the mailbox inaccessible through the linear mapping hence the need for ioremap_cache()

Patch 2/3 is also a fix for a potential issue on UEFI boot, but it is unrelated to 1/3 and 3/3

Quoted 8 lines
Quoted 4 lines

I was planning to take all of these for 3.18 as there's no regression here (the fuzzing is a new debug feature and defaults to `n'). Do you think these qualify as -rc1 material?

Considering that TEXT_OFFSET fuzzing is recommended to be turned on for distro kernels, I would say this is definitely appropriate for 3.17

Quoted 6 lines

Whilst I see that in the commit log, the same recommendation doesn't appear in the Kconfig text and I'm not sure that it's such a wise thing to say either. From a distribution's point of view, I think I'd want any kernel issues to be as reproducible as possible, and fuzzing the text offset seems to go against that.

OK. There is one other real world issue that 3/3 addresses, which are platforms that have memreserves at the base of DRAM containing bits of UEFI itself (this is what got this whole discussion going in the first place). Currently, we cannot boot via UEFI on these platforms, as the EFI stub will only consider base of DRAM + TEXT_OFFSET as an option, and fail the boot if it is not available.

If this is something that could wait until 3.18 as well (Mark S?), then it's fine by me.

Signature · 1 line
Ard
#8 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 42 lines · Ard Biesheuvel
Quoted 27 lines · Will Deacon
Quoted 17 lines · Ard Biesheuvel
Quoted 16 lines · Will Deacon
Quoted 14 lines · Ard Biesheuvel

Patch 3/3 makes the EFI stub load Image at the lowest 2 meg boundary + TEXT_OFFSET instead of erroring out when base of DRAM + TEXT_OFFSET is occupied. This is necessary as the randomized TEXT_OFFSET could potentially conflict with reserved areas at the base of DRAM if a low value happens to be chosen.

Patch 1/3 fixes the resulting breakage on APM Mustang in bringing up the secondaries, as on that board in particular, the reserved area at the base of DRAM contains the holding pen, and loading Image higher up makes the mailbox inaccessible through the linear mapping hence the need for ioremap_cache()

Patch 2/3 is also a fix for a potential issue on UEFI boot, but it is unrelated to 1/3 and 3/3

Quoted 8 lines
Quoted 4 lines

I was planning to take all of these for 3.18 as there's no regression here (the fuzzing is a new debug feature and defaults to `n'). Do you think these qualify as -rc1 material?

Considering that TEXT_OFFSET fuzzing is recommended to be turned on for distro kernels, I would say this is definitely appropriate for 3.17

Quoted 6 lines

Whilst I see that in the commit log, the same recommendation doesn't appear in the Kconfig text and I'm not sure that it's such a wise thing to say either. From a distribution's point of view, I think I'd want any kernel issues to be as reproducible as possible, and fuzzing the text offset seems to go against that.

OK. There is one other real world issue that 3/3 addresses, which are platforms that have memreserves at the base of DRAM containing bits of UEFI itself (this is what got this whole discussion going in the first place). Currently, we cannot boot via UEFI on these platforms, as the EFI stub will only consider base of DRAM + TEXT_OFFSET as an option, and fail the boot if it is not available.

Quoted 3 lines

If this is something that could wait until 3.18 as well (Mark S?), then it's fine by me.

I don't see a problem with waiting until 3.18.

#9 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 27 lines · Mark Salter
Quoted 26 lines · Ard Biesheuvel
Quoted 11 lines · Will Deacon
Quoted 9 lines · Ard Biesheuvel
Quoted 4 lines · Will Deacon

I was planning to take all of these for 3.18 as there's no regression here (the fuzzing is a new debug feature and defaults to `n'). Do you think these qualify as -rc1 material?

Considering that TEXT_OFFSET fuzzing is recommended to be turned on for distro kernels, I would say this is definitely appropriate for 3.17

Quoted 6 lines

Whilst I see that in the commit log, the same recommendation doesn't appear in the Kconfig text and I'm not sure that it's such a wise thing to say either. From a distribution's point of view, I think I'd want any kernel issues to be as reproducible as possible, and fuzzing the text offset seems to go against that.

OK. There is one other real world issue that 3/3 addresses, which are platforms that have memreserves at the base of DRAM containing bits of UEFI itself (this is what got this whole discussion going in the first place). Currently, we cannot boot via UEFI on these platforms, as the EFI stub will only consider base of DRAM + TEXT_OFFSET as an option, and fail the boot if it is not available.

Quoted 5 lines
Quoted 3 lines

If this is something that could wait until 3.18 as well (Mark S?), then it's fine by me.

I don't see a problem with waiting until 3.18.

Cheers, I've applied the series to our for-next branch and I'll push everything out at -rc1.

Will

#10 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 27 lines · Will Deacon
Quoted 26 lines · Mark Salter
Quoted 25 lines · Ard Biesheuvel
Quoted 11 lines · Will Deacon
Quoted 9 lines · Ard Biesheuvel
Quoted 4 lines · Will Deacon

I was planning to take all of these for 3.18 as there's no regression here (the fuzzing is a new debug feature and defaults to `n'). Do you think these qualify as -rc1 material?

Considering that TEXT_OFFSET fuzzing is recommended to be turned on for distro kernels, I would say this is definitely appropriate for 3.17

Quoted 5 lines

Whilst I see that in the commit log, the same recommendation doesn't appear in the Kconfig text and I'm not sure that it's such a wise thing to say either. From a distribution's point of view, I think I'd want any kernel issues to be as reproducible as possible, and fuzzing the text offset seems to go against that.

OK. There is one other real world issue that 3/3 addresses, which are platforms that have memreserves at the base of DRAM containing bits of UEFI itself (this is what got this whole discussion going in the first place). Currently, we cannot boot via UEFI on these platforms, as the EFI stub will only consider base of DRAM + TEXT_OFFSET as an option, and fail the boot if it is not available.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

If this is something that could wait until 3.18 as well (Mark S?), then it's fine by me.

I don't see a problem with waiting until 3.18.

Cheers, I've applied the series to our for-next branch and I'll push everything out at -rc1.

Late to the discussion since I was on an 11h flight, and still chiming in since I don't think previous speakers were explicit enough about the implication:
Bumping 3/3 to 3.18 means the upstream kernel will not boot on AMD Seattle until at least December. This sounds suboptimal to me.

/
Leif

#11 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 32 lines · Leif Lindholm
Quoted 31 lines · Will Deacon
Quoted 29 lines · Mark Salter
Quoted 24 lines · Ard Biesheuvel
Quoted 10 lines · Will Deacon
Quoted 8 lines · Ard Biesheuvel
Quoted 3 lines · Will Deacon

I was planning to take all of these for 3.18 as there's no regression here (the fuzzing is a new debug feature and defaults to `n'). Do you think these qualify as -rc1 material?

Considering that TEXT_OFFSET fuzzing is recommended to be turned on for distro kernels, I would say this is definitely appropriate for 3.17

Quoted 5 lines

Whilst I see that in the commit log, the same recommendation doesn't appear in the Kconfig text and I'm not sure that it's such a wise thing to say either. From a distribution's point of view, I think I'd want any kernel issues to be as reproducible as possible, and fuzzing the text offset seems to go against that.

OK. There is one other real world issue that 3/3 addresses, which are platforms that have memreserves at the base of DRAM containing bits of UEFI itself (this is what got this whole discussion going in the first place). Currently, we cannot boot via UEFI on these platforms, as the EFI stub will only consider base of DRAM + TEXT_OFFSET as an option, and fail the boot if it is not available.

Quoted 2 lines

If this is something that could wait until 3.18 as well (Mark S?), then it's fine by me.

I don't see a problem with waiting until 3.18.

Quoted 8 lines
Quoted 2 lines

Cheers, I've applied the series to our for-next branch and I'll push everything out at -rc1.

Late to the discussion since I was on an 11h flight, and still chiming in since I don't think previous speakers were explicit enough about the implication:
Bumping 3/3 to 3.18 means the upstream kernel will not boot on AMD Seattle until at least December. This sounds suboptimal to me.

Not unless they use PSCI. That's already working (the spin-table is meant for platforms where PSCI cannot be implemented).

BTW, is Seattle officially supported by the mainline kernel (it was barely announced AFAICT)?

Signature · 1 line
Catalin
#12 lore raw

PATCH v2 1/3 arm64: spin-table: handle unmapped cpu-release-addrs

Quoted 36 lines · Catalin Marinas
Quoted 35 lines · Leif Lindholm
Quoted 33 lines · Will Deacon
Quoted 25 lines · Mark Salter
Quoted 24 lines · Ard Biesheuvel
Quoted 10 lines · Will Deacon
Quoted 8 lines · Ard Biesheuvel
Quoted 3 lines · Will Deacon

I was planning to take all of these for 3.18 as there's no regression here (the fuzzing is a new debug feature and defaults to `n'). Do you think these qualify as -rc1 material?

Considering that TEXT_OFFSET fuzzing is recommended to be turned on for distro kernels, I would say this is definitely appropriate for 3.17

Quoted 5 lines

Whilst I see that in the commit log, the same recommendation doesn't appear in the Kconfig text and I'm not sure that it's such a wise thing to say either. From a distribution's point of view, I think I'd want any kernel issues to be as reproducible as possible, and fuzzing the text offset seems to go against that.

OK. There is one other real world issue that 3/3 addresses, which are platforms that have memreserves at the base of DRAM containing bits of UEFI itself (this is what got this whole discussion going in the first place). Currently, we cannot boot via UEFI on these platforms, as the EFI stub will only consider base of DRAM + TEXT_OFFSET as an option, and fail the boot if it is not available.

Quoted 4 lines
Quoted 2 lines

If this is something that could wait until 3.18 as well (Mark S?), then it's fine by me.

I don't see a problem with waiting until 3.18.

Cheers, I've applied the series to our for-next branch and I'll push everything out at -rc1.

Quoted 9 lines
Quoted 5 lines

Late to the discussion since I was on an 11h flight, and still chiming in since I don't think previous speakers were explicit enough about the implication:
Bumping 3/3 to 3.18 means the upstream kernel will not boot on AMD Seattle until at least December. This sounds suboptimal to me.

Not unless they use PSCI. That's already working (the spin-table is meant for platforms where PSCI cannot be implemented).

No, that is patch #1

#3 allows other base addresses than base of DRAM + TEXT_OFFSET when booting via UEFI, which is needed by AMD Seattle. However, #3 breaks booting the secondaries on APM Mustang, hence #1.

Signature · 1 line
Ard.
Quoted 5 lines

BTW, is Seattle officially supported by the mainline kernel (it was barely announced AFAICT)?

--
Catalin