[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH gnumach] Add vm_pages_phys
From: |
Samuel Thibault |
Subject: |
Re: [PATCH gnumach] Add vm_pages_phys |
Date: |
Tue, 30 Jan 2024 19:45:31 +0100 |
User-agent: |
NeoMutt/20170609 (1.8.3) |
Sergey Bugaev, le mar. 30 janv. 2024 09:38:39 +0300, a ecrit:
> On Mon, Jan 29, 2024 at 11:59 PM Samuel Thibault
> <samuel.thibault@gnu.org> wrote:
> > Please notably review the RPC part, I really don't know that much about
> > mig.
>
> Some nitpicks inline. Flávio, does what I'm saying below make sense to you?
>
> > +/*
> > + * vm_pages_phys returns information about a region of memory
> > + */
> > +kern_return_t vm_pages_phys(
> > + host_t host,
> > + vm_map_t map,
> > + vm_address_t address,
> > + vm_size_t size,
>
> This is supposed to be a number of pages rather than VM region size,
> right?
No, I followed the same principle as the vm_allocate etc. calls.
In the function itself, count = atop(size).
> > + rpc_phys_addr_array_t *pagespp,
> > + mach_msg_type_number_t *countp)
> > +{
> > + if (host == HOST_NULL)
> > + return KERN_INVALID_HOST;
> > + if (map == VM_MAP_NULL)
> > + return KERN_INVALID_TASK;
> > +
> > + if (!page_aligned(address))
> > + return KERN_INVALID_ARGUMENT;
> > + if (!page_aligned(size))
> > + return KERN_INVALID_ARGUMENT;
> > +
> > + mach_msg_type_number_t count = atop(size), cur;
> > +
> > + if (*countp < count)
> > + return KERN_INVALID_ARGUMENT;
>
> It's not an error if the passed-in buffer is smaller than what you're
> planning to return.
Right, I was lazy here :) and indeed the 256 static count can pose
problem.
> You're supposed to allocate your own buffer in this case, something like this:
Ok, done so, thanks!
> OK. My current understanding is that you indeed do not need to
> deallocate either the 'vm_map_t map' argument or the task port right
> behind it on the success path here. But perhaps it would be clearer
> with a comment stating that this is intentional (and not simply
> forgotten).
This is the case in all RPC calls of the kernel :)
Samuel