Re: umm_map returns unaligned address?

2021-04-23 Thread Alessandro Pistocchi
Sorry, I didn't see previous replies because the emails went to the spam
folder
for some reason.

I didn't mean to waste anybody's time. Apologies. I replied to your
subsequent points below
and I'll send the diff in a new email.

On Sat, Apr 24, 2021 at 2:24 AM Philip Guenther  wrote:

> On Fri, Apr 23, 2021 at 3:13 PM Alessandro Pistocchi <
> apukbusin...@gmail.com> wrote:
>
>> -- Forwarded message -
>> From: Alessandro Pistocchi 
>> Date: Fri, Apr 23, 2021 at 1:55 PM
>> Subject: umm_map returns unaligned address?
>> To: 
>>
>>
>> Hi all,
>>
>> I am fairly new to openbsd so if this is something obvious that I missed
>> please be understanding.
>>
>> I am adding a syscall to openbsd 6.8. I am working on a raspberry pi.
>>
>> During the syscall I allocate some memory that I want to share between the
>> kernel
>> and the calling process.
>>
>> When it's time to wrap up and unmap the memory, I unmap it both from the
>> kernel
>> map and from the process map.
>>
>> The unmapping from the process map goes fine, the unmapping from the
>> kernel
>> map
>> fails by saying that the virtual address in kernel map is not aligned to
>> the page size
>> ( it's actually 4 bytes off ).
>>
>> What have I missed? I assumed that umm_map would return a page aligned
>> virtual
>> address for the kernel mapping as well.
>>
>> Here is my code for creating the shared memory chunk:
>>
>
> Stop sending summaries and just send diffs that compile: you don't know
> everything that is relevant and keep leaving out stuff that is.  I'm the
> third person to say this.
>
>
>>
>> 
>> // memory_size is a multiple of page size
>> uvm_object = uao_create(memory_size, 0);
>> if(!uvm_object) return;
>>
>> // TODO(ale): make sure that this memory cannot be swapped out
>>
>> uao_reference(uvm_object)
>> if(uvm_map(kernel_map, (vaddr_t *)&memory, round_page(memory_size),
>> uvm_object,
>>0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ | PROT_WRITE,
>>MAP_INHERIT_SHARED, MADV_NORMAL, 0))) {
>>
>
> The cast of &memory is wrong: it's either unnecessary (if memory is of the
> correct type) or totally broken (if it isn't).  Why did you think it was
> unnecessary to show how you declared your variables?
>

You're right.
memory is a void * which is zero at call time, vadd_t an unsigned long. I
believed it was ok since I am expecting to receive the address of the start
of the mapping.


>
> You also fail to show your initialization of 'memory'.  If you didn't then
> that's ABSOLUTELY wrong and not in line with the existing uses of uvm_map()
> in the kernel.  Please consult the uvm_map(9) manpage for what the incoming
> value means.
>
> ...
>
>> uao_reference(uvm_object);
>> if(uvm_map(&p->p_vmspace->vm_map, &memory_in_proc_space,
>> round_page(memory_size), uvm_object,
>>0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ | PROT_WRITE,
>>MAP_INHERIT_NONE, MADV_NORMAL, 0))) {
>> memory = 0;
>>
>
> This error handling is incomplete, lacking an unmap.
>

Yep, I had noticed that and corrected it in my code. However that should
not be related to the problem I get.


>
>
> Philip Guenther
>


Re: umm_map returns unaligned address?

2021-04-23 Thread Philip Guenther
On Fri, Apr 23, 2021 at 3:13 PM Alessandro Pistocchi 
wrote:

> -- Forwarded message -
> From: Alessandro Pistocchi 
> Date: Fri, Apr 23, 2021 at 1:55 PM
> Subject: umm_map returns unaligned address?
> To: 
>
>
> Hi all,
>
> I am fairly new to openbsd so if this is something obvious that I missed
> please be understanding.
>
> I am adding a syscall to openbsd 6.8. I am working on a raspberry pi.
>
> During the syscall I allocate some memory that I want to share between the
> kernel
> and the calling process.
>
> When it's time to wrap up and unmap the memory, I unmap it both from the
> kernel
> map and from the process map.
>
> The unmapping from the process map goes fine, the unmapping from the kernel
> map
> fails by saying that the virtual address in kernel map is not aligned to
> the page size
> ( it's actually 4 bytes off ).
>
> What have I missed? I assumed that umm_map would return a page aligned
> virtual
> address for the kernel mapping as well.
>
> Here is my code for creating the shared memory chunk:
>

Stop sending summaries and just send diffs that compile: you don't know
everything that is relevant and keep leaving out stuff that is.  I'm the
third person to say this.


>
> 
> // memory_size is a multiple of page size
> uvm_object = uao_create(memory_size, 0);
> if(!uvm_object) return;
>
> // TODO(ale): make sure that this memory cannot be swapped out
>
> uao_reference(uvm_object)
> if(uvm_map(kernel_map, (vaddr_t *)&memory, round_page(memory_size),
> uvm_object,
>0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ | PROT_WRITE,
>MAP_INHERIT_SHARED, MADV_NORMAL, 0))) {
>

The cast of &memory is wrong: it's either unnecessary (if memory is of the
correct type) or totally broken (if it isn't).  Why did you think it was
unnecessary to show how you declared your variables?

You also fail to show your initialization of 'memory'.  If you didn't then
that's ABSOLUTELY wrong and not in line with the existing uses of uvm_map()
in the kernel.  Please consult the uvm_map(9) manpage for what the incoming
value means.

...

> uao_reference(uvm_object);
> if(uvm_map(&p->p_vmspace->vm_map, &memory_in_proc_space,
> round_page(memory_size), uvm_object,
>0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ | PROT_WRITE,
>MAP_INHERIT_NONE, MADV_NORMAL, 0))) {
> memory = 0;
>

This error handling is incomplete, lacking an unmap.


Philip Guenther