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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 14 Feb 2022 15:03:32 +0200
From:   Oded Gabbay <ogabbay@...nel.org>
To:     Hillf Danton <hdanton@...a.com>
Cc:     Ohad Sharabi <osharabi@...ana.ai>,
        "Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] habanalabs: allow user to set allocation page size

On Mon, Feb 14, 2022 at 3:02 PM Oded Gabbay <ogabbay@...nel.org> wrote:
>
> On Mon, Feb 14, 2022 at 12:48 PM Hillf Danton <hdanton@...a.com> 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;