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

[Oded Gabbay]

On Mon, Feb 14, 2022 at 3:02 PM Oded Gabbay <ogabbay@xxxxxxxxxx> wrote:
>
> 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.
This will be in a separate patch.
Oded

>
>
> >
> > >               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;