Re: [U-Boot] [PATCH 1/5] arch/powerpc/cpu/mpc8xxx: PAMU driver support

On Thu, Mar 28, 2013 at 5:46 AM, Ruchika Gupta ruchika.gupta@freescale.comwrote:

PAMU driver basic support for usage in Secure Boot. In secure boot PAMU is not in bypass mode. Hence to use any peripheral (SEC Job ring in our case), PAMU has to be configured.

The Header file fsl_pamu.h and few functions in driver have been derived from Freescale Libos.

Signed-off-by: Kuldip Giroh kuldip.giroh@freescale.com Signed-off-by: Ruchika Gupta ruchika.gupta@freescale.com

---

Based upon git://git.denx.de/u-boot.git branch master

You don't really have to say this as a) It's assumed, b) It becomes quickly obsolete (as it is now). You probably only need to mention it if you're based on an unusual git repo.

diff --git a/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c b/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c new file mode 100644 index 0000000..bdbec07 --- /dev/null +++ b/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c

[...]

+ +static inline int __ilog2_roundup_64(uint64_t val) +{ + + if ((val & (val - 1)) == 0) + return __ilog2_u64(val); + else + return __ilog2_u64(val) + 1; +}

These sorts of functions seem like they belong in arch/powerpc/include/asm/bitops.h

+static inline int count_lsb_zeroes(unsigned long val) +{

•   return ffs(val) - 1;
  

+}

[...]

+static int pamu_config_ppaace(uint32_t liodn, uint64_t win_addr,

•   uint64_t win_size, uint32_t omi,
  

•   uint32_t snoopid, uint32_t stashid,
  

•   uint32_t subwin_cnt)
  

+{

•   unsigned long fspi;
  

•   paace_t *ppaace;
  

•   if ((win_size & (win_size - 1)) || win_size < PAMU_PAGE_SIZE)
  

•           return -1;
  

•   if (win_addr & (win_size - 1))
  

•           return -2;
  

•   if (liodn > NUM_PPAACT_ENTRIES) {
  

•           printf("Entries in PPACT not sufficient\n");
  

•           return -3;
  

•   }
  
  

These return values should really have names to define, up-front, what they mean.

•   ppaace = &ppaact[liodn];
  

•   /* window size is 2^(WSE+1) bytes */
  

•   set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE,
  

•           map_addrspace_size_to_wse(win_size));
  

•   pamu_setup_default_xfer_to_host_ppaace(ppaace);
  

•   if (sizeof(phys_addr_t) > 4)
  

•           ppaace->wbah = (u64)win_addr >> (PAMU_PAGE_SHIFT + 20);
  

•   else
  

•           ppaace->wbah = 0;
  

•   set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL,
  

•          (win_addr >> PAMU_PAGE_SHIFT));
  

•   /* set up operation mapping if it's configured */
  

•   if (omi < OME_NUMBER_ENTRIES) {
  

•           set_bf(ppaace->impl_attr, PAACE_IA_OTM, PAACE_OTM_INDEXED);
  

•           ppaace->op_encode.index_ot.omi = omi;
  

•   } else if (~omi != 0)
  

•           return -3;
  
  

ditto here

+static int pamu_config_spaace(uint32_t liodn,

•   uint64_t subwin_size, uint64_t subwin_addr, uint64_t size,
  

•   uint32_t omi, uint32_t snoopid, uint32_t stashid)
  

+{

•   paace_t *paace;
  

•   unsigned long fspi;
  

•   /* Align start addr of subwin to subwindoe size */
  

•   uint64_t sec_addr = subwin_addr & ~(subwin_size - 1);
  

•   uint64_t end_addr = subwin_addr + size;
  

•   int size_shift = __ilog2_u64(subwin_size);
  

•   uint64_t win_size = 0;
  

•   uint32_t index, swse;
  

•   /* Recalculate the size */
  

•   size = end_addr - sec_addr;
  

•   if (!subwin_size)
  

•           return -1;
  
  

Also need a named constant here

•   if (liodn > NUM_PPAACT_ENTRIES) {
  

•           printf("LIODN Number to be programmed %d"
  

•                   "greater than number of PPAACT entries %d\n",
  

•                           liodn, NUM_PPAACT_ENTRIES);
  

•           return -1;
  

•   }
  
  

And here

•   while (sec_addr < end_addr) {
  

+#ifdef DEBUG

•           printf("sec_addr < end_addr is %llx < %llx\n", sec_addr,
  

•                   end_addr);
  

+#endif

•           paace = &ppaact[liodn];
  

•           if (!paace)
  

•                   return -1;
  
  

And here

+ +int pamu_init(void) +{ + u32 base_addr = CONFIG_SYS_PAMU_ADDR;

void *base_addr = (void *)CONFIG_SYS_PAMU_ADDR;

+ ccsr_pamu_t *regs; + u32 i = 0; + u64 ppaact_phys, ppaact_lim, ppaact_size; + u64 spaact_phys, spaact_lim, spaact_size; + + ppaact_size = sizeof(paace_t) * NUM_PPAACT_ENTRIES; + spaact_size = sizeof(paace_t) * NUM_SPAACT_ENTRIES; + + /* Allocate space for Primary PAACT Table */ + ppaact = memalign(PAMU_TABLE_ALIGNMENT, ppaact_size); + if (!ppaact) + return -1;

Named constant.

•   memset(ppaact, 0, ppaact_size);
  

•   /* Allocate space for Secondary PAACT Table */
  

•   sec = memalign(PAMU_TABLE_ALIGNMENT, spaact_size);
  

•   if (!sec)
  

•           return -1;
  
  

And here

•   memset(sec, 0, spaact_size);
  

•   ppaact_phys = virt_to_phys((void *)ppaact);
  

•   ppaact_lim = ppaact_phys + ppaact_size;
  

•   spaact_phys = (uint64_t)virt_to_phys((void *)sec);
  

•   spaact_lim = spaact_phys + spaact_size;
  

•   /* Configure all PAMU's */
  

•   for (i = 0; i < CONFIG_NUM_PAMU; i++) {
  

•           regs = (ccsr_pamu_t *)base_addr;
  
  

No need for cast once base_addr is more sane.

•           out_be32(&regs->ppbah, ppaact_phys >> 32);
  

•           out_be32(&regs->ppbal, (uint32_t)ppaact_phys);
  

•           out_be32(&regs->pplah, (ppaact_lim) >> 32);
  

•           out_be32(&regs->pplal, (uint32_t)ppaact_lim);
  

•           if (sec != NULL) {
  

•                   out_be32(&regs->spbah, spaact_phys >> 32);
  

•                   out_be32(&regs->spbal, (uint32_t)spaact_phys);
  

•                   out_be32(&regs->splah, spaact_lim >> 32);
  

•                   out_be32(&regs->splal, (uint32_t)spaact_lim);
  

•           }
  

•           asm volatile("sync" : : : "memory");
  

•           base_addr += PAMU_OFFSET;
  

•   }
  

•   return 0;
  

+}

+void pamu_enable(void) +{

•   u32 i = 0;
  

•   u32 base_addr = CONFIG_SYS_PAMU_ADDR;
  
  

void *base_addr = (void *)CONFIG_SYS_PAMU_ADDR;

•   for (i = 0; i < CONFIG_NUM_PAMU; i++) {
  

•           setbits_be32((void *)base_addr + PAMU_PCR_OFFSET,
  

•                   PAMU_PCR_PE);
  
  

And then eliminate the "(void *)" here

•           asm volatile("sync" : : : "memory");
  

•           base_addr += PAMU_OFFSET;
  

•   }
  

+}

+void pamu_reset(void) +{

•   u32 i  = 0;
  

•   u32 base_addr = CONFIG_SYS_PAMU_ADDR;
  
  

void *base_addr = (void *)CONFIG_SYS_PAMU_ADDR;

In general, why would you declare an address as u32? We have 32 and 64-bit processors that use PAMU. Does this work at all on e5500 or e6500?

+ ccsr_pamu_t *regs;

•   for (i = 0; i < CONFIG_NUM_PAMU; i++) {
  

•           regs = (ccsr_pamu_t *)base_addr;
  
  

Get rid of cast by following above recommendation.

•   /* Clear PPAACT Base register */
  

•           out_be32(&regs->ppbah, 0);
  

•           out_be32(&regs->ppbal, 0);
  

•           out_be32(&regs->pplah, 0);
  

•           out_be32(&regs->pplal, 0);
  

•           out_be32(&regs->spbah, 0);
  

•           out_be32(&regs->spbal, 0);
  

•           out_be32(&regs->splah, 0);
  

•           out_be32(&regs->splal, 0);
  

•           clrbits_be32((void *)regs + PAMU_PCR_OFFSET, PAMU_PCR_PE);
  

•           asm volatile("sync" : : : "memory");
  

•           base_addr += PAMU_OFFSET;
  

•   }
  

+}

+void pamu_disable(void) +{

•   u32 i  = 0;
  

•   u32 base_addr = CONFIG_SYS_PAMU_ADDR;
  
  

ditto here.

•   for (i = 0; i < CONFIG_NUM_PAMU; i++) {
  

•           clrbits_be32((void *)base_addr + PAMU_PCR_OFFSET,
  

PAMU_PCR_PE);

And this cast

•           asm volatile("sync" : : : "memory");
  

•           base_addr += PAMU_OFFSET;
  

•   }
  

+}

[...]

+int config_pamu(struct pamu_addr_tbl *tbl, int num_entries, uint32_t liodn) +{

•   int i = 0;
  

•   int ret = 0;
  

•   uint32_t num_sec_windows = 0;
  

•   uint32_t num_windows = 0;
  

•   uint64_t min_addr, max_addr;
  

•   uint64_t size;
  

•   uint64_t subwin_size;
  

•   int sizebit;
  

•   min_addr = find_min(tbl->start_addr, num_entries);
  

•   max_addr = find_max(tbl->end_addr, num_entries);
  

•   size = max_addr - min_addr + 1;
  

•   if (!size)
  

•           return -1;
  
  

Named constant

•   sizebit = __ilog2_roundup_64(size);
  

•   size = 1 << sizebit;
  

+#ifdef DEBUG

•   printf("min start_addr is %llx\n", min_addr);
  

•   printf("max end_addr is %llx\n", max_addr);
  

•   printf("size found is  %llx\n", size);
  

+#endif

•   if (size < PAMU_PAGE_SIZE)
  

•           size = PAMU_PAGE_SIZE;
  

•   while (1) {
  

•           min_addr = min_addr & ~(size - 1);
  

•           if (min_addr + size > max_addr)
  

•                   break;
  

•           size <<= 1;
  

•           if (!size)
  

•                   return -1;
  
  

Named constant

diff --git a/arch/powerpc/include/asm/fsl_pamu.h b/arch/powerpc/include/asm/fsl_pamu.h new file mode 100644 index 0000000..e2cfe97 --- /dev/null +++ b/arch/powerpc/include/asm/fsl_pamu.h @@ -0,0 +1,193 @@

+#define CONFIG_NUM_PAMU 16 +#define NUM_PPAACT_ENTRIES 512 +#define NUM_SPAACT_ENTRIES 128

Aren't these values variable, depending on the platform?

+/* PAMU_OFFSET to the next pamu space in ccsr */ +#define PAMU_OFFSET 0x1000

+#define PAMU_TABLE_ALIGNMENT 0x00001000

+#define PAMU_PAGE_SHIFT 12 +#define PAMU_PAGE_SIZE 4096U

+#define PAACE_M_COHERENCE_REQ 0x01

+#define PAACE_DA_HOST_CR 0x80 +#define PAACE_DA_HOST_CR_SHIFT 7

+#define PAACE_AF_PT 0x00000002 +#define PAACE_AF_PT_SHIFT 1

+#define PAACE_PT_PRIMARY 0x0 +#define PAACE_PT_SECONDARY 0x1

+#define PPAACE_AF_WBAL 0xfffff000 +#define PPAACE_AF_WBAL_SHIFT 12

+#define OME_NUMBER_ENTRIES 16 /* based on P4080 2.0 silicon plan */

...Is this P4080-specific?

Andy