Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

boards: xenvm: remove incorrect condition for Kconfig heap values #81790

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

boards: xenvm: remove incorrect condition for Kconfig heap values #81790

wants to merge 2 commits into from
+11 −4

Conversation

firscity
Copy link
Contributor

xenvm Kconfig contained incorrect name for board parameter. It led to build issues - heap size was set incorrectly. Since whole file is already placed under right Kconfig condition ("if BOARD_XENVM"), remove incorret parameter at all.

This issue was introduced by commit 8dc3f85 during migration to hwmv2 due to typo.

@firscity firscity requested a review from lorc November 22, 2024 17:26
@zephyrbot zephyrbot added size: XS A PR changing only a single line of code area: Xen Platform labels Nov 22, 2024
@firscity
boards: xenvm: remove incorrect condition for Kconfig heap values
fab0cd6 
xenvm Kconfig contained incorrect name for board parameter. It led to
build issues - heap size was set incorrectly. Since whole file is
already placed under right Kconfig condition ("if BOARD_XENVM"), remove
incorrect parameter at all.

This issue was introduced by commit 8dc3f85 ("hwmv2: Introduce
Hardware model version 2 and convert devices") due to the typo.

Signed-off-by: Dmytro Firsov <[email protected]>
luca-fancellu
luca-fancellu previously approved these changes Nov 22, 2024
View reviewed changes
lorc
lorc previously approved these changes Nov 22, 2024
View reviewed changes
kartben
kartben previously approved these changes Nov 22, 2024
View reviewed changes
nordicjm
nordicjm previously approved these changes Nov 25, 2024
View reviewed changes
@kartben
Copy link
Collaborator

kartben commented Nov 25, 2024

@firscity it looks like the CI failures might be legit?

@firscity
Copy link
Contributor Author

@kartben yeah, thank you
fixing of this issue highlighted another, I am currently trying to find best way to fix both in scope of this PR

kartben
kartben requested changes Nov 25, 2024
View reviewed changes
tests/kernel/device/testcase.yaml Outdated
@@ -7,7 +7,7 @@ tests:
kernel.device:
integration_platforms:
- native_sim
platform_exclude: xenvm
platform_exclude: xenvm xenvm/xenvm/gicv3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make all of these multiline please, this way diffs will be cleaner going forward

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for suggestion, done.

@firscity
tests: kernel: exclude xenvm with GICv3 from device tests
aea83bc 
The issue with mentioned test for GICv3 version of xenvm virtual
board is the same as for regular xenvm - device tree overlay for this
test overrides #address-cells and #size-cells property for original
device tree file (from 0x2 to 0x1), which leads to incorrect DT parsing
by actual Xen drivers. This causes build errors, so test should be
skipped for GICv3 platform too.

Same issue for regular xenvm was fixed by commit 40fe366 ("tests:
kernel: exclude xenvm from device tests"). Issue for GICv3 appeared
after migrating to HWMv2.

Signed-off-by: Dmytro Firsov <[email protected]>
@firscity
Copy link
Contributor Author

Thanks everyone for review and approves.

For everyone previously reviewed:
Adding changes to xenvm board triggered another issue with HWMv2 where invalid test were applied to newly introduced (by HWMv2) board. Additional commit added this board to excludes as it was made for xenvm.

Also I have no idea why net.mqtt_sn.packet test is failing, it doesn't look like smth that where caused by my changes.

@firscity
Copy link
Contributor Author

Hi @rlubos, as I can see, you are maintainer of net.mqtt_sn.packet test. It is failing unexpectedly during build, can you please check it? It looks like something not related to this PR.

@kartben
Copy link
Collaborator

kartben commented Nov 27, 2024

Hi @rlubos, as I can see, you are maintainer of net.mqtt_sn.packet test. It is failing unexpectedly during build, can you please check it? It looks like something not related to this PR.

This has been fixed in main. Please rebase to make the error go away:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lorc lorc lorc approved these changes

@kartben kartben kartben approved these changes

@luca-fancellu luca-fancellu luca-fancellu left review comments

@nordicjm nordicjm nordicjm left review comments

@povergoing povergoing Awaiting requested review from povergoing

@SgrrZhf SgrrZhf Awaiting requested review from SgrrZhf

@andyross andyross Awaiting requested review from andyross

@ceolin ceolin Awaiting requested review from ceolin

@cfriedt cfriedt Awaiting requested review from cfriedt

@dcpleung dcpleung Awaiting requested review from dcpleung

@gmarull gmarull Awaiting requested review from gmarull

@nashif nashif Awaiting requested review from nashif

@npitre npitre Awaiting requested review from npitre

@peter-mitsis peter-mitsis Awaiting requested review from peter-mitsis

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

Assignees

@povergoing povergoing

Labels
area: Device Model area: Kernel area: Xen Platform size: XS A PR changing only a single line of code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@firscity @kartben @lorc @nordicjm @luca-fancellu @povergoing @zephyrbot