Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

nvdimm: support nvdimm on arm64 kernel #377

Merged
merged 1 commit into from
Mar 12, 2019
Merged

nvdimm: support nvdimm on arm64 kernel #377

merged 1 commit into from
Mar 12, 2019

Conversation

Pennyzct
Copy link
Contributor

@Pennyzct Pennyzct commented Mar 5, 2019

let's open nvdimm-related kernel config parameters on arm64, such as
CONFIG_ACPI_NFIT, etc. and we also need to backport patch kvm:arm64:Dynamic IPA and 52bit IPA and related dependency into v4.19.X to fully support nvdimm from guest kernel.
Former patch has already been merged into v4.20.X.

Fixes: #376

Signed-off-by: Penny Zheng [email protected]

@Pennyzct
Copy link
Contributor Author

Pennyzct commented Mar 5, 2019 •
edited
Loading

Hi~ @grahamwhaley @chavafg Is there any way to trigger ARM CI here? ;)

@Pennyzct Pennyzct mentioned this pull request Mar 5, 2019
Merged
grahamwhaley
grahamwhaley approved these changes Mar 5, 2019
View reviewed changes
Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

A couple of questions, but generally
lgtm

kernel/configs/arm64_kata_kvm_4.19.x Outdated
@@ -1141,6 +1175,13 @@ CONFIG_OF_NET=y
CONFIG_OF_RESERVED_MEM=y
# CONFIG_OF_OVERLAY is not set
# CONFIG_PARPORT is not set
CONFIG_PNP=y
CONFIG_PNP_DEBUG_MESSAGES=y
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if you set this deliberately? Payoff of size vs debuggability...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

disable it. ;)

kernel/configs/arm64_kata_kvm_4.19.x Outdated
# CONFIG_THERMAL_GOV_FAIR_SHARE is not set
# CONFIG_THERMAL_GOV_STEP_WISE is not set
# CONFIG_THERMAL_GOV_BANG_BANG is not set
CONFIG_THERMAL_GOV_USER_SPACE=y
Copy link
Contributor

Choose a reason for hiding this comment

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

On purpose? Maybe you gained some default =y items during a new config generation. If you have not already, you might want to scrutinize the diff for any stray new additions you didn't intend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noooo, my bad, I just quickly compared the new diff with x86 config(x86_64_kata_kvm_4.19.X) to make two configs as same as possible, not scrutinizing what these diff represent.
I will turn off the CONFIG_THERMAL, since VM may not be involved in thermal managment. ;)

@grahamwhaley
Copy link
Contributor

@Pennyzct - no, no way to trigger the ARM CI here - in fact, we tried to add the Power8 CI here last week iirc (or maybe that was osbuilder), and it didn't work as it went to pull OBS images that did not exist I think.
But, @chavafg , might there be a way to trigger a PR on another repo that has this PR as a dependency, to get it pulled in? I'm not sure..

@grahamwhaley
Copy link
Contributor

ooh, 'leaky pods' on the 18.04 CI....
@nitkon - did you get any further on that, or are you now needing direction/assistance?

bash sanity/check_sanity.sh
ERROR: 3 pods left and found at /var/lib/vc/sbs
Makefile:45: recipe for target 'functional' failed
make: *** [functional] Error 1
Build step 'Execute shell' marked build as failure

@nitkon
Copy link
Contributor

nitkon commented Mar 5, 2019

Yes, @jodh-intel suggested to get a kernel backtrace of such VM Panics. We can probably get that by enabling serial console and then redirecting the guest serial to some host file.
However reproducing this issue locally would be the first challenge as running these tests individually don't result in panic(In my testing atleast)

@jodh-intel
Copy link
Contributor

@nitkon - you should get something in the proxy log without the need for a serial console. Certainly oops's are logged an I've seen panics when the rootfs is not mountable.

@Pennyzct
Copy link
Contributor Author

Pennyzct commented Mar 6, 2019

Hi~ @grahamwhaley I had another relevant PR in runtime repo, and i will try to make this pr as dependency to see if ARM CI on runtime repo could test these two PRs together. ;)

@Pennyzct Pennyzct force-pushed the nvdimm branch 2 times, most recently from efa99dd to 8570097 Compare March 7, 2019 03:10
@Pennyzct
Copy link
Contributor Author

Pennyzct commented Mar 7, 2019

Hi~@grahamwhaley I have addressed the comments and done the fix, ptal. ;)
Also, I have already done the re-base and update the kata_config_version.
For now, I'm struggling to add this PR as dependency for another related PR runtime/#1323.
So I could trigger the ARM CI under runtime/ repo to test this two PRs together. I definitely once saw how to make this happen, like dependency: packaging/#377, I just couldn't find the reference, would anyone like to lend a hand here? ;) @jodh-intel @chavafg

@jodh-intel
Copy link
Contributor

Hi @Pennyzct - It's actually documented here:

And here's an example of a PR that used it:

Pennyzct added a commit to Pennyzct/runtime that referenced this pull request Mar 7, 2019
@Pennyzct
qemu-arm64: Support nvdimm on arm64
986e4dc 
Original guest image was reprensented as block device in qemu-aarch64,
and it will bring up write lock error when running multiple containers.
Thanks to the new expanded IPA_SIZE feature in kernel 4.20 and
Eric Auger's related patch set in qemu(which are still under upstream
review), we could fully support nvdimm on arm64.

Depends-on: github.com/kata-containers/packaging#377

Fixes: kata-containers#843

Signed-off-by: Penny Zheng <[email protected]>
Pennyzct added a commit to Pennyzct/runtime that referenced this pull request Mar 7, 2019
@Pennyzct
qemu-arm64: add unit test for func appendImage on aarch64
5b1bf8b 
Since we overrided the func appendImage for aarch64, we should also
provide related unit test.

Depends-on: github.com/kata-containers/packaging#377

Fixes: kata-containers#843

Signed-off-by: Penny Zheng <[email protected]>
@Pennyzct
Copy link
Contributor Author

Pennyzct commented Mar 7, 2019

Hi~ @jodh-intel tons of thanks!!!
I have already add dependencies in PR runtime/#1323, let's see how it works~~~~;)

@amshinde
Copy link
Member

/test

@amshinde
Copy link
Member

amshinde commented Mar 11, 2019 •
edited
Loading

Still seeing failures:

Kernel directory has changes check kernel/kata_config_version changed
ERROR: Please bump version in kernel/kata_config_version

In spite of the version being changed. Retriggering one more time...

/test

@Pennyzct
nvdimm: support nvdimm on arm64 kernel
5dd1d7c 
let's open nvdimm-related kernel config parameters on arm64, such as
CONFIG_ACPI_NFIT, etc. and we also need to backport patch
'kvm:arm64:Dynamic IPA and 52bit IPA'(https://patchwork.kernel.org/cover/10616271/)
and related dependency into v4.19.X to fully support nvdimm from guest kernel.
Former patch has already been merged into v4.20.X.

Fixes: kata-containers#376

Signed-off-by: Penny Zheng <[email protected]>
@Pennyzct
Copy link
Contributor Author

Pennyzct commented Mar 12, 2019 •
edited
Loading

Hi~ @amshinde re-based and updated the version number. ;)

@jodh-intel
Copy link
Contributor

Thanks @Pennyzct. Let's retry the CI...

/retest

@jodh-intel jodh-intel merged commit 9a8553f into kata-containers:master Mar 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@grahamwhaley grahamwhaley grahamwhaley approved these changes

@devimc devimc devimc approved these changes

@jodh-intel jodh-intel jodh-intel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Pennyzct @grahamwhaley @nitkon @jodh-intel @amshinde @devimc