linux-kernel - RE: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <C5ECD7A89D1DC44195F34B25E172658D3C2691@039-SN2MPN1-013.039d.mgd.msft.net>
Date:	Wed, 27 Feb 2013 11:56:08 +0000
From:	Sethi Varun-B16395 <B16395@...escale.com>
To:	Stuart Yoder <b08248@...il.com>
CC:	"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Wood Scott-B07421 <B07421@...escale.com>,
	Joerg Roedel <joro@...tes.org>,
	Yoder Stuart-B08248 <B08248@...escale.com>
Subject: RE: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API
 implementation.



> -----Original Message-----
> From: Stuart Yoder [mailto:b08248@...il.com]
> Sent: Wednesday, February 27, 2013 4:03 AM
> To: Sethi Varun-B16395
> Cc: iommu@...ts.linux-foundation.org; linuxppc-dev@...ts.ozlabs.org;
> linux-kernel@...r.kernel.org; Wood Scott-B07421; Joerg Roedel; Yoder
> Stuart-B08248
> Subject: Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU
> API implementation.
> 
> Have not got through the entire file, but have a few comments...
> 
> +/*
> + * Set the PAACE type as primary and set the coherency required domain
> + * attribute
> + */
> +static void pamu_setup_default_xfer_to_host_ppaace(struct paace
> +*ppaace) {
> +       set_bf(ppaace->addr_bitfields, PAACE_AF_PT, PAACE_PT_PRIMARY);
> +
> +       set_bf(ppaace->domain_attr.to_host.coherency_required,
> PAACE_DA_HOST_CR,
> +              PAACE_M_COHERENCE_REQ);
> +}
> +
> +/*
> + * Set the PAACE type as secondary and set the coherency required
> +domain
> + * attribute.
> + */
> +static void pamu_setup_default_xfer_to_host_spaace(struct paace
> +*spaace) {
> +       set_bf(spaace->addr_bitfields, PAACE_AF_PT, PAACE_PT_SECONDARY);
> +       set_bf(spaace->domain_attr.to_host.coherency_required,
> PAACE_DA_HOST_CR,
> +              PAACE_M_COHERENCE_REQ);
> +}
> 
> Can we change the names of the above functions...   I know there is some
> history
> with the name, but "xfer_to_host" is confusing.
> 
> Maybe just call them:
> 
> pamu_init_paace()
> pamu_init_spaace()
> 
[Sethi Varun-B16395] ok, will change the function names.

> > +/**
> > + * pamu_config_spaace() - Sets up SPAACE entry for specified
> > +subwindow
> > + *
> > + * @liodn:  Logical IO device number
> > + * @subwin_cnt:  number of sub-windows associated with dma-window
> > + * @subwin_addr: starting address of subwindow
> > + * @subwin_size: size of subwindow
> > + * @omi: Operation mapping index
> > + * @rpn: real (true physical) page number
> > + * @snoopid: snoop id for hardware coherency -- if ~snoopid == 0 then
> > + *                       snoopid not defined
> > + * @stashid: cache stash id for associated cpu
> > + * @enable: enable/disable subwindow after reconfiguration
> > + * @prot: sub window permissions
> > + *
> > + * Returns 0 upon success else error code < 0 returned  */ int
> > +pamu_config_spaace(int liodn, u32 subwin_cnt, u32 subwin_addr,
> > +                      phys_addr_t subwin_size, u32 omi, unsigned long
> rpn,
> > +                      u32 snoopid, u32 stashid, int enable, int prot)
> > +{
> > +       struct paace *paace;
> > +
> > +       /* setup sub-windows */
> > +       if (!subwin_cnt) {
> > +               pr_err("Invalid subwindow count\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       paace = pamu_get_ppaace(liodn);
> > +       if (subwin_addr > 0 && subwin_addr < subwin_cnt && paace) {
> 
> Why is the comparison subwin_addr < subwin_cnt?   Seems wrong...
> 
[Sethi Varun-B16395] It's actually the subwindow index. I will rename the variable.

> > +               paace = pamu_get_spaace(paace, subwin_addr - 1);
> > +
> > +               if (paace && !(paace->addr_bitfields & PAACE_V_VALID))
> {
> > +                       pamu_setup_default_xfer_to_host_spaace(paace);
> > +                       set_bf(paace->addr_bitfields, SPAACE_AF_LIODN,
> liodn);
> > +               }
> > +       }
> > +
> > +       if (!paace) {
> > +               pr_err("Invalid liodn entry\n");
> > +               return -ENOENT;
> > +       }
> > +
> > +       if (!enable && prot == PAACE_AP_PERMS_DENIED) {
> > +               if (subwin_addr > 0)
> > +                       set_bf(paace->addr_bitfields, PAACE_AF_V,
> > +                                PAACE_V_INVALID);
> > +               else
> > +                       set_bf(paace->addr_bitfields, PAACE_AF_AP,
> > +                                prot);
> > +               mb();
> > +               return 0;
> > +       }
> 
> Can you add a comment to the above if statement...when is this function
> called with PAACE_AP_PERMS_DENIED?
> 
[Sethi Varun-B16395] Actually, this piece of code is redundant in case of the window based API. I will remove this. PAACE_AP_PERMS_DENIED is primarily used for disabling the primary subwindow.

-Varun


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/