Re: [PATCH 2/2] habanalabs: allow user to set allocation page size

[Oded Gabbay]

On Mon, Feb 14, 2022 at 12:48 PM Hillf Danton <hdanton@xxxxxxxx> wrote:
>
> On Mon, 14 Feb 2022 10:12:10 +0200 Ohad Sharabi wrote:
> >
> >  struct hl_mem_in {
> >       union {
> > -             /* HL_MEM_OP_ALLOC- allocate device memory */
> > +             /**
> > +              * structure for device memory allocation (used with the HL_MEM_OP_ALLOC op)
> > +              * @mem_size: memory size to allocate
> > +              * @page_size: page size to use on allocation. when the value is 0 the default page
> > +              *             size will be taken.
> > +              */
>
> Nit, the 80-column-char format is prfered.
Maybe it is preferred, but afaik it is now allowed to use 100-chars,
and checkpatch no longer shouts about
it, so I don't see the harm.
>
> Wonder how to get the available page sizes in userspace?
We will add an opcode to our INFO ioctl to return the available page sizes.
The user will need to query this at the beginning of the application run.


>
> >               struct {
> > -                     /* Size to alloc */
> >                       __u64 mem_size;
> > +                     __u64 page_size;
> >               } alloc;
> >
> > -             /* HL_MEM_OP_FREE - free device memory */
> > +             /**
> > +              * structure for free-ing device memory (used with the HL_MEM_OP_FREE op)
> > +              * @handle: handle returned from HL_MEM_OP_ALLOC
>
> Nit, add a seperate cleanup patch for changes with no link to page size.
Yes, correct, I will split this up.

Thanks,
Oded
>
> > +              */
> >               struct {
> > -                     /* Handle returned from HL_MEM_OP_ALLOC */
> >                       __u64 handle;
> >               } free;
> >
> > -             /* HL_MEM_OP_MAP - map device memory */
> > +             /**
> > +              * structure for mapping device memory (used with the HL_MEM_OP_MAP op)
> > +              * @hint_addr: requested virtual address of mapped memory.
> > +              *             the driver will try to map the requested region to this hint
> > +              *             address, as long as the address is valid and not already mapped.
> > +              *             the user should check the returned address of the IOCTL to make
> > +              *             sure he got the hint address.
> > +              *             passing 0 here means that the driver will choose the address itself.
> > +              * @handle: handle returned from HL_MEM_OP_ALLOC.
> > +              */
> >               struct {
> > -                     /*
> > -                      * Requested virtual address of mapped memory.
> > -                      * The driver will try to map the requested region to
> > -                      * this hint address, as long as the address is valid
> > -                      * and not already mapped. The user should check the
> > -                      * returned address of the IOCTL to make sure he got
> > -                      * the hint address. Passing 0 here means that the
> > -                      * driver will choose the address itself.
> > -                      */
> >                       __u64 hint_addr;
> > -                     /* Handle returned from HL_MEM_OP_ALLOC */
> >                       __u64 handle;
> >               } map_device;