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.

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

tests: kernel: App to validate boot time page table #1286

Merged
merged 1 commit into from
Dec 5, 2017

Conversation

akhileshupadhyay
Copy link

Testcase developed to validate boot time page table faults.

Signed-off-by: Akhilesh Kumar Upadhyay [email protected]

galak
galak previously requested changes Aug 29, 2017
Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

This seems like a pretty x86 specific test, can we rename so its clear that its x86 specific.

Copy link
Member

@nashif nashif left a comment

Choose a reason for hiding this comment

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

Can you please add a README explaining what is being tested here and give more details?

@@ -0,0 +1,13 @@
#include <ztest.h>
Copy link
Member

Choose a reason for hiding this comment

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

missing copyright/license header

@@ -0,0 +1,28 @@
#ifndef X86_MMU_GET_PT_ADDR
Copy link
Member

Choose a reason for hiding this comment

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

missing copyright/license header

Copy link
Member

Choose a reason for hiding this comment

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

you also need a header guard here

@@ -0,0 +1,110 @@
#include <zephyr.h>
Copy link
Member

Choose a reason for hiding this comment

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

missing copyright/license header

@akhileshupadhyay
Copy link
Author

Missing copyright/license header is added to the necessary files. README file is added with details. The header guards are also added.

#define STARTING_ADDR_RANGE_LMT 0x0009ff
#define START_ADR_RANGE_OVRLP_LMT 0x001000
#define REGION_PERM (MMU_READ_WRITE | MMU_PAGE_USER)
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

move line 37 (#endif )to line 27

Copy link
Author

Choose a reason for hiding this comment

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

#endif moved to line 27

#include <ztest.h>
#include "boot_page_table.h"

MMU_BOOT_REGION(START_ADDR_RANGE, ADDR_SIZE, REGION_PERM);
Copy link
Contributor

Choose a reason for hiding this comment

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

add multiple regions into the test. as of now it is testing a single memory region.
Create multiple memory regions and validate them with the below APIs.

Copy link
Author

Choose a reason for hiding this comment

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

multiple memory regions has been created and added to validate upon.

@akhileshupadhyay akhileshupadhyay force-pushed the bootpagetable branch 3 times, most recently from 02e4f9c to d69c2bc Compare September 5, 2017 13:18
zassert_true(ending_start_addr_range() == TC_PASS, NULL);
}

void test_starting_addr_range1(void)
Copy link
Contributor

Choose a reason for hiding this comment

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

Pass arguments into the same function. if arguments== 1 then take start addr 1 if argument == 2 then take start addr2 so on.
Don't duplicate the code.

Copy link
Author

Choose a reason for hiding this comment

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

@AdithyaBaglody . The memory address are already passed based on the arguments.

nagineni pushed a commit to nagineni/zephyr that referenced this pull request Nov 20, 2017
…ct-rtos#1286)

This includes base configuration common to all platforms.

Fixes zephyrproject-rtos#1260.

Signed-off-by: Geoff Gustafson <[email protected]>
@akhileshupadhyay akhileshupadhyay force-pushed the bootpagetable branch 7 times, most recently from 5899ded to 85e5da1 Compare November 27, 2017 08:13
@akhileshupadhyay
Copy link
Author

@galak Could you please review the changes requested by you.

Copy link

@kumarvikash1 kumarvikash1 left a comment

Choose a reason for hiding this comment

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

copyright/license header, header guard and line breaking has been addressed. Approved.

@kumarvikash1
Copy link

@SebastianBoe @galak : Can you please review the changes. This PR is planned for R-1.10.

@SebastianBoe SebastianBoe removed their request for review November 30, 2017 10:57
@SebastianBoe SebastianBoe removed their assignment Nov 30, 2017
@SebastianBoe
Copy link
Collaborator

I am not able to review, removed myself from reviewers list.

@galak
Copy link
Collaborator

galak commented Nov 30, 2017

My comment about renaming since this is x86 specific is still valid.

@akhileshupadhyay
Copy link
Author

@galak . Could you please check, the renaming is done for x86.

@@ -0,0 +1,4 @@
tests:
- test:
platform_whitelist: qemu_x86
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any reason this test will not run on any x86 platform?

Seems like we should have:

arch_whitelist: x86

for sure in here.

@akhileshupadhyay
Copy link
Author

@galak . The changes have been addressed for arch_whitelist: x86

@galak galak dismissed their stale review December 1, 2017 15:37

Changes made that I asked for

@galak
Copy link
Collaborator

galak commented Dec 1, 2017

@akhileshupadhyay thanks. I removed my 'requests changes'. I leave this up to @nashif to decide if he's ok with it and wants it merged in 1.10.

@akhileshupadhyay akhileshupadhyay force-pushed the bootpagetable branch 2 times, most recently from 3e73924 to 761e7b0 Compare December 4, 2017 06:35
Copy link
Member

@nashif nashif left a comment

Choose a reason for hiding this comment

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

remove x86 from the path name x86boot_page_table/...

Testcase developed to validate x86 specific boot time page table faults.

Signed-off-by: Akhilesh Kumar Upadhyay <[email protected]>
@akhileshupadhyay
Copy link
Author

@nashif Could please check, removed x86 from x86boot_page_table path name.

@nashif nashif merged commit 149c341 into zephyrproject-rtos:master Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants