| Message ID | 20200611161500.23580-2-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | SMMUv3.2 Range-based TLB Invalidation Support | expand |
On Thu, 11 Jun 2020 at 17:15, Eric Auger <eric.auger@redhat.com> wrote: > > Page and block PTE decoding can share some code. Let's > first handle table PTE and factorize some code shared by > page and block PTEs. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > --- > hw/arm/smmu-common.c | 51 ++++++++++++++++---------------------------- > 1 file changed, 18 insertions(+), 33 deletions(-) > > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c > index e13a5f4a7c..f2de2be527 100644 > --- a/hw/arm/smmu-common.c > +++ b/hw/arm/smmu-common.c > @@ -186,12 +186,12 @@ static int smmu_ptw_64(SMMUTransCfg *cfg, > uint64_t subpage_size = 1ULL << level_shift(level, granule_sz); > uint64_t mask = subpage_size - 1; > uint32_t offset = iova_level_offset(iova, inputsize, level, granule_sz); > - uint64_t pte; > + uint64_t pte, gpa; > dma_addr_t pte_addr = baseaddr + offset * sizeof(pte); > uint8_t ap; > > if (get_pte(baseaddr, offset, &pte, info)) { > - goto error; > + break; get_pte() fills in info->type (to SMMU_PTW_ERR_WALK_EABT) on error; changing this from "goto error" to "break" means we'll now execute the "info->type = SMMU_PTW_ERR_TRANSLATION" that comes between the end of the while loop and the error: label, overwriting the wrong error type. thanks -- PMM
Hi Peter, On 6/25/20 4:49 PM, Peter Maydell wrote: > On Thu, 11 Jun 2020 at 17:15, Eric Auger <eric.auger@redhat.com> wrote: >> >> Page and block PTE decoding can share some code. Let's >> first handle table PTE and factorize some code shared by >> page and block PTEs. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> --- >> hw/arm/smmu-common.c | 51 ++++++++++++++++---------------------------- >> 1 file changed, 18 insertions(+), 33 deletions(-) >> >> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c >> index e13a5f4a7c..f2de2be527 100644 >> --- a/hw/arm/smmu-common.c >> +++ b/hw/arm/smmu-common.c >> @@ -186,12 +186,12 @@ static int smmu_ptw_64(SMMUTransCfg *cfg, >> uint64_t subpage_size = 1ULL << level_shift(level, granule_sz); >> uint64_t mask = subpage_size - 1; >> uint32_t offset = iova_level_offset(iova, inputsize, level, granule_sz); >> - uint64_t pte; >> + uint64_t pte, gpa; >> dma_addr_t pte_addr = baseaddr + offset * sizeof(pte); >> uint8_t ap; >> >> if (get_pte(baseaddr, offset, &pte, info)) { >> - goto error; >> + break; > > get_pte() fills in info->type (to SMMU_PTW_ERR_WALK_EABT) on > error; changing this from "goto error" to "break" means we'll > now execute the "info->type = SMMU_PTW_ERR_TRANSLATION" that > comes between the end of the while loop and the error: label, > overwriting the wrong error type. Agreed. Thanks Eric > > thanks > -- PMM >
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index e13a5f4a7c..f2de2be527 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -186,12 +186,12 @@ static int smmu_ptw_64(SMMUTransCfg *cfg, uint64_t subpage_size = 1ULL << level_shift(level, granule_sz); uint64_t mask = subpage_size - 1; uint32_t offset = iova_level_offset(iova, inputsize, level, granule_sz); - uint64_t pte; + uint64_t pte, gpa; dma_addr_t pte_addr = baseaddr + offset * sizeof(pte); uint8_t ap; if (get_pte(baseaddr, offset, &pte, info)) { - goto error; + break; } trace_smmu_ptw_level(level, iova, subpage_size, baseaddr, offset, pte); @@ -199,58 +199,43 @@ static int smmu_ptw_64(SMMUTransCfg *cfg, if (is_invalid_pte(pte) || is_reserved_pte(pte, level)) { trace_smmu_ptw_invalid_pte(stage, level, baseaddr, pte_addr, offset, pte); - info->type = SMMU_PTW_ERR_TRANSLATION; - goto error; + break; } - if (is_page_pte(pte, level)) { - uint64_t gpa = get_page_pte_address(pte, granule_sz); + if (is_table_pte(pte, level)) { + ap = PTE_APTABLE(pte); - ap = PTE_AP(pte); if (is_permission_fault(ap, perm)) { info->type = SMMU_PTW_ERR_PERMISSION; goto error; } - - tlbe->translated_addr = gpa + (iova & mask); - tlbe->perm = PTE_AP_TO_PERM(ap); + baseaddr = get_table_pte_address(pte, granule_sz); + level++; + continue; + } else if (is_page_pte(pte, level)) { + gpa = get_page_pte_address(pte, granule_sz); trace_smmu_ptw_page_pte(stage, level, iova, baseaddr, pte_addr, pte, gpa); - return 0; - } - if (is_block_pte(pte, level)) { + } else { uint64_t block_size; - hwaddr gpa = get_block_pte_address(pte, level, granule_sz, - &block_size); - - ap = PTE_AP(pte); - if (is_permission_fault(ap, perm)) { - info->type = SMMU_PTW_ERR_PERMISSION; - goto error; - } + gpa = get_block_pte_address(pte, level, granule_sz, + &block_size); trace_smmu_ptw_block_pte(stage, level, baseaddr, pte_addr, pte, iova, gpa, block_size >> 20); - - tlbe->translated_addr = gpa + (iova & mask); - tlbe->perm = PTE_AP_TO_PERM(ap); - return 0; } - - /* table pte */ - ap = PTE_APTABLE(pte); - + ap = PTE_AP(pte); if (is_permission_fault(ap, perm)) { info->type = SMMU_PTW_ERR_PERMISSION; goto error; } - baseaddr = get_table_pte_address(pte, granule_sz); - level++; + + tlbe->translated_addr = gpa + (iova & mask); + tlbe->perm = PTE_AP_TO_PERM(ap); + return 0; } - info->type = SMMU_PTW_ERR_TRANSLATION; - error: tlbe->perm = IOMMU_NONE; return -EINVAL;
Page and block PTE decoding can share some code. Let's first handle table PTE and factorize some code shared by page and block PTEs. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- hw/arm/smmu-common.c | 51 ++++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 33 deletions(-)