From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrii Tseglytskyi Subject: Re: [PATCH v02 2/7] arm: omap: introduce iommu translation for IPU remoteproc Date: Tue, 22 Jul 2014 19:58:18 +0300 Message-ID: References: <1403780826-22123-1-git-send-email-andrii.tseglytskyi@globallogic.com> <1403780826-22123-3-git-send-email-andrii.tseglytskyi@globallogic.com> <53B6BA7B.8090806@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53B6BA7B.8090806@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org Hi Julien, On Fri, Jul 4, 2014 at 5:30 PM, Julien Grall wrote: > Hi Andrii, > > > On 26/06/14 12:07, Andrii Tseglytskyi wrote: >> >> +static u32 mmu_pte_table_alloc(struct mmu_info *mmu, u32 pgd, u32 >> sect_num, >> + struct mmu_pagetable *pgt, u32 hyp_addr) >> +{ >> + u32 *pte = NULL; >> + u32 i; >> + >> + /* allocate pte table once */ >> + if ( 0 == hyp_addr ) >> + { >> + pte = xzalloc_bytes(PAGE_SIZE); >> + if ( !pte ) >> + { >> + pr_mmu("failed to alloc 2nd level table"); >> + return 0; >> + } >> + } >> + else >> + { >> + pte = __va(hyp_addr & MMU_SECTION_MASK(mmu->pg_data->pte_shift)); >> + } >> + >> + ASSERT(256 == MMU_PTRS_PER_PTE(mmu)); >> + >> + for ( i = 0; i < MMU_PTRS_PER_PTE(mmu); i++ ) >> + { >> + u32 paddr, maddr; >> + >> + paddr = pgd + (i * PAGE_SIZE); >> + maddr = p2m_lookup(current->domain, paddr, NULL); >> + ASSERT(maddr != INVALID_PADDR); > > > Hmmm, what prevents the guest to pass an invalid IPA? On debug build you > will hit the assert, but on non-debug build you will screw the IPU page > table. > Agree. Will replace ASSERTS with proper if - else logic. > [..] > >> +static u32 mmu_ipu_translate_pagetable(struct mmu_info *mmu, struct >> mmu_pagetable *pgt) >> +{ > > > [..] > > >> + /* first level pointers have different formats, depending on >> their type */ >> + if ( ipu_pgd_is_super(pgd) ) >> + pd_mask = MMU_SECTION_MASK(MMU_OMAP_SUPER_SHIFT); >> + else if ( ipu_pgd_is_section(pgd) ) >> + pd_mask = MMU_SECTION_MASK(MMU_OMAP_SECTION_SHIFT); >> + else if ( ipu_pgd_is_table(pgd) ) >> + pd_mask = MMU_SECTION_MASK(MMU_OMAP_SECOND_LEVEL_SHIFT); >> + >> + pd_paddr = pgd & pd_mask; >> + pd_flags = pgd & ~pd_mask; >> + pd_maddr = p2m_lookup(current->domain, pd_paddr, NULL); >> + ASSERT(pd_maddr != INVALID_PADDR); > > > Same remark here. > OK > -- > Julien Grall > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel -- Andrii Tseglytskyi | Embedded Dev GlobalLogic www.globallogic.com