target/microblaze: Add memattrs on transactions

[PATCH v1 3/3] target/microblaze: Add security attributes on memory transactions

Posted by Joe Komlodi 4 years, 10 months ago

Using the cfg.use_non_secure bitfield and the MMU access type, we can determine
if the access should be secure or not.

Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
---
 target/microblaze/cpu.c    |  2 +-
 target/microblaze/cpu.h    |  3 ++-
 target/microblaze/helper.c | 26 +++++++++++++++++++++++---
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index accfb23..d5e8bfe 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -375,7 +375,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
     cc->tlb_fill = mb_cpu_tlb_fill;
 #ifndef CONFIG_USER_ONLY
     cc->do_transaction_failed = mb_cpu_transaction_failed;
-    cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
+    cc->get_phys_page_attrs_debug = mb_cpu_get_phys_page_attrs_debug;
     dc->vmsd = &vmstate_mb_cpu;
 #endif
     device_class_set_props(dc, mb_properties);
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index 199cfb0..e4bba8a 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -361,7 +361,8 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
                                 MMUAccessType access_type,
                                 int mmu_idx, uintptr_t retaddr);
 void mb_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
-hwaddr mb_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
+hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
+                                        MemTxAttrs *attrs);
 int mb_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int mb_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 
diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index cda14a1..20dbd67 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -46,6 +46,16 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 
 #else /* !CONFIG_USER_ONLY */
 
+static bool mb_cpu_access_is_secure(MicroBlazeCPU *cpu,
+                                    MMUAccessType access_type)
+{
+    if (access_type == MMU_INST_FETCH) {
+        return !cpu->ns_axi_ip;
+    } else {
+        return !cpu->ns_axi_dp;
+    }
+}
+
 bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                      MMUAccessType access_type, int mmu_idx,
                      bool probe, uintptr_t retaddr)
@@ -55,12 +65,16 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     MicroBlazeMMULookup lu;
     unsigned int hit;
     int prot;
+    MemTxAttrs attrs = {};
+
+    attrs.secure = mb_cpu_access_is_secure(cpu, access_type);
 
     if (mmu_idx == MMU_NOMMU_IDX) {
         /* MMU disabled or not available.  */
         address &= TARGET_PAGE_MASK;
         prot = PAGE_BITS;
-        tlb_set_page(cs, address, address, prot, mmu_idx, TARGET_PAGE_SIZE);
+        tlb_set_page_with_attrs(cs, address, address, attrs, prot, mmu_idx,
+                                TARGET_PAGE_SIZE);
         return true;
     }
 
@@ -71,7 +85,8 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 
         qemu_log_mask(CPU_LOG_MMU, "MMU map mmu=%d v=%x p=%x prot=%x\n",
                       mmu_idx, vaddr, paddr, lu.prot);
-        tlb_set_page(cs, vaddr, paddr, lu.prot, mmu_idx, TARGET_PAGE_SIZE);
+        tlb_set_page_with_attrs(cs, vaddr, paddr, attrs, lu.prot, mmu_idx,
+                                TARGET_PAGE_SIZE);
         return true;
     }
 
@@ -230,7 +245,8 @@ void mb_cpu_do_interrupt(CPUState *cs)
     }
 }
 
-hwaddr mb_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
+hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
+                                        MemTxAttrs *attrs)
 {
     MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
     CPUMBState *env = &cpu->env;
@@ -239,6 +255,10 @@ hwaddr mb_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
     int mmu_idx = cpu_mmu_index(env, false);
     unsigned int hit;
 
+    /* Caller doesn't initialize */
+    *attrs = (MemTxAttrs) {};
+    attrs->secure = mb_cpu_access_is_secure(cpu, MMU_DATA_LOAD);
+
     if (mmu_idx != MMU_NOMMU_IDX) {
         hit = mmu_translate(cpu, &lu, addr, 0, 0);
         if (hit) {
-- 
2.7.4

Re: [PATCH v1 3/3] target/microblaze: Add security attributes on memory transactions

Posted by Edgar E. Iglesias 4 years, 9 months ago

On Thu, Jan 21, 2021 at 04:18:55PM -0800, Joe Komlodi wrote:
> Using the cfg.use_non_secure bitfield and the MMU access type, we can determine
> if the access should be secure or not.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> 
> Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> ---
>  target/microblaze/cpu.c    |  2 +-
>  target/microblaze/cpu.h    |  3 ++-
>  target/microblaze/helper.c | 26 +++++++++++++++++++++++---
>  3 files changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index accfb23..d5e8bfe 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -375,7 +375,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
>      cc->tlb_fill = mb_cpu_tlb_fill;
>  #ifndef CONFIG_USER_ONLY
>      cc->do_transaction_failed = mb_cpu_transaction_failed;
> -    cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
> +    cc->get_phys_page_attrs_debug = mb_cpu_get_phys_page_attrs_debug;
>      dc->vmsd = &vmstate_mb_cpu;
>  #endif
>      device_class_set_props(dc, mb_properties);
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index 199cfb0..e4bba8a 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -361,7 +361,8 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
>                                  MMUAccessType access_type,
>                                  int mmu_idx, uintptr_t retaddr);
>  void mb_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
> -hwaddr mb_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> +hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
> +                                        MemTxAttrs *attrs);
>  int mb_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>  int mb_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>  
> diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
> index cda14a1..20dbd67 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/helper.c
> @@ -46,6 +46,16 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>  
>  #else /* !CONFIG_USER_ONLY */
>  
> +static bool mb_cpu_access_is_secure(MicroBlazeCPU *cpu,
> +                                    MMUAccessType access_type)
> +{
> +    if (access_type == MMU_INST_FETCH) {
> +        return !cpu->ns_axi_ip;
> +    } else {
> +        return !cpu->ns_axi_dp;
> +    }
> +}
> +
>  bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>                       MMUAccessType access_type, int mmu_idx,
>                       bool probe, uintptr_t retaddr)
> @@ -55,12 +65,16 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>      MicroBlazeMMULookup lu;
>      unsigned int hit;
>      int prot;
> +    MemTxAttrs attrs = {};
> +
> +    attrs.secure = mb_cpu_access_is_secure(cpu, access_type);
>  
>      if (mmu_idx == MMU_NOMMU_IDX) {
>          /* MMU disabled or not available.  */
>          address &= TARGET_PAGE_MASK;
>          prot = PAGE_BITS;
> -        tlb_set_page(cs, address, address, prot, mmu_idx, TARGET_PAGE_SIZE);
> +        tlb_set_page_with_attrs(cs, address, address, attrs, prot, mmu_idx,
> +                                TARGET_PAGE_SIZE);
>          return true;
>      }
>  
> @@ -71,7 +85,8 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>  
>          qemu_log_mask(CPU_LOG_MMU, "MMU map mmu=%d v=%x p=%x prot=%x\n",
>                        mmu_idx, vaddr, paddr, lu.prot);
> -        tlb_set_page(cs, vaddr, paddr, lu.prot, mmu_idx, TARGET_PAGE_SIZE);
> +        tlb_set_page_with_attrs(cs, vaddr, paddr, attrs, lu.prot, mmu_idx,
> +                                TARGET_PAGE_SIZE);
>          return true;
>      }
>  
> @@ -230,7 +245,8 @@ void mb_cpu_do_interrupt(CPUState *cs)
>      }
>  }
>  
> -hwaddr mb_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> +hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
> +                                        MemTxAttrs *attrs)
>  {
>      MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
>      CPUMBState *env = &cpu->env;
> @@ -239,6 +255,10 @@ hwaddr mb_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>      int mmu_idx = cpu_mmu_index(env, false);
>      unsigned int hit;
>  
> +    /* Caller doesn't initialize */
> +    *attrs = (MemTxAttrs) {};
> +    attrs->secure = mb_cpu_access_is_secure(cpu, MMU_DATA_LOAD);
> +
>      if (mmu_idx != MMU_NOMMU_IDX) {
>          hit = mmu_translate(cpu, &lu, addr, 0, 0);
>          if (hit) {
> -- 
> 2.7.4
>

[PATCH v1 1/3] target/microblaze: Add use-non-secure property
[PATCH v1 2/3] target/microblaze: use MMUAccessType instead of int in mmu_translate
[PATCH v1 3/3] target/microblaze: Add security attributes on memory transactions