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

RISC-V port fixes (part I) #2518

Merged
merged 2 commits into from
Nov 21, 2024
Merged

RISC-V port fixes (part I) #2518

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a0e0c1
compel: fix gitignore and remove autogenerated code
mihalicyn Nov 17, 2024
ec22c38
compel/arch/riscv64: properly implement compel_task_size()
mihalicyn Nov 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions compel/arch/riscv64/src/lib/infect.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,20 +181,22 @@ int arch_fetch_sas(struct parasite_ctl *ctl, struct rt_sigframe *s)
* Task size is the maximum virtual address space size that a process can occupy in the memory
* Refer to linux kernel arch/riscv/include/asm/pgtable.h,
* task size is:
* - 0x9fc00000 (~2.5GB) for RV32.
* - 0x4000000000 ( 256GB) for RV64 using SV39 mmu
* - 0x800000000000 ( 128TB) for RV64 using SV48 mmu
*
* Note that PGDIR_SIZE must evenly divide TASK_SIZE since "RISC-V
* Instruction Set Manual Volume II: Privileged Architecture" states that
* "load and store effective addresses, which are 64bits, must have bits
* 63–48 all equal to bit 47, or else a page-fault exception will occur."
*/
#define TASK_SIZE 0x800000000000UL // hardcoded for SV48 MMU
* - 0x9fc00000 (~2.5GB) for RV32.
* - 0x4000000000 ( 256GB) for RV64 using SV39 mmu
* - 0x800000000000 ( 128TB) for RV64 using SV48 mmu
* - 0x100000000000000 ( 64PB) for RV64 using SV57 mmu
*/
#define TASK_SIZE_MIN (1UL << 38)
#define TASK_SIZE_MAX (1UL << 56)

unsigned long compel_task_size(void)
{
return TASK_SIZE;
unsigned long task_size;

for (task_size = TASK_SIZE_MIN; task_size < TASK_SIZE_MAX; task_size <<= 1)
if (munmap((void *)task_size, page_size()))
Copy link
Member

Choose a reason for hiding this comment

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

I merged it because I was thinking that I'd reviewed it already.

I think this munamp can unmap something useful and trigger sigsegv.

Copy link
Member Author

@mihalicyn mihalicyn Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, actually, we are using the same approach for other architectures:

criu/compel/arch/arm/src/lib/infect.c

Line 184 in 1452c76

unsigned long compel_task_size(void)

criu/compel/arch/ppc64/src/lib/infect.c

Line 507 in 1452c76

unsigned long compel_task_size(void)

But that's an interesting point and probably I should try to find a cleaner way to determine a task size across all architectures we have then.

But taking into account that it's not a new and dirty hack I introduced but rather reusing an existing and time-proven method I think it's good enough to be there.

Upd: A first thing that comes into my mind is to replace munmap with some "safer" syscall like madvise. But it's still requires investigation...

Upd: we can also do mmap before munmap.

Copy link
Member

Choose a reason for hiding this comment

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

We can do something like this: https://cs.opensource.google/gvisor/gvisor/+/master:pkg/abi/linux/mm.go;l=142?q=tasksize&ss=gvisor%2Fgvisor

break;
return task_size;
}

/*
Expand Down
Loading