diff mbox

[RFC,for-4.8,v2,2/7] xen/arm: Rename and generalize un/map_regions_rw_cache

Message ID 1464960552-6645-3-git-send-email-edgar.iglesias@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Edgar E. Iglesias June 3, 2016, 1:29 p.m. UTC 
From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Rename and generalize un/map_regions_rw_cache into
un/map_regions. The new functions take the mapping
attributes and access permissions as arguments.

No functional change.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 xen/arch/arm/domain_build.c | 18 ++++++++++--------
 xen/arch/arm/p2m.c          | 28 ++++++++++++++++------------
 xen/include/asm-arm/p2m.h   | 22 +++++++++++++---------
 3 files changed, 39 insertions(+), 29 deletions(-)

Comments

Julien Grall June 6, 2016, 5:55 p.m. UTC | #1 
Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Rename and generalize un/map_regions_rw_cache into
> un/map_regions.

I would name it map_regions_mattr (or something similar) to stop people 
using this helper to map real RAM.

> The new functions take the mapping
> attributes and access permissions as arguments.

I overlooked it when I reviewed {,un}map_regions_rw_cache. p2m_access_t 
is for memaccess to restrict the permission. The helper should always 
use d->arch.p2m.default_access.

Can you fix it in a separate patch and request for backport to Xen 4.7?

Regards,
Edgar E. Iglesias June 7, 2016, 8:55 p.m. UTC | #2 
On Mon, Jun 06, 2016 at 06:55:30PM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 03/06/16 14:29, Edgar E. Iglesias wrote:
> >From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> >Rename and generalize un/map_regions_rw_cache into
> >un/map_regions.
> 
> I would name it map_regions_mattr (or something similar) to stop people
> using this helper to map real RAM.

Sounds good, I've done it for v3.

> 
> >The new functions take the mapping
> >attributes and access permissions as arguments.
> 
> I overlooked it when I reviewed {,un}map_regions_rw_cache. p2m_access_t is
> for memaccess to restrict the permission. The helper should always use
> d->arch.p2m.default_access.
> 
> Can you fix it in a separate patch and request for backport to Xen 4.7?

Yes, I'll do that.

Cheers,
Edgar


> 
> Regards,
> 
> -- 
> Julien Grall
diff mbox

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 00dc07a..e4fed4b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1570,10 +1570,11 @@  static void acpi_map_other_tables(struct domain *d)
     {
         addr = acpi_gbl_root_table_list.tables[i].address;
         size = acpi_gbl_root_table_list.tables[i].length;
-        res = map_regions_rw_cache(d,
-                                   paddr_to_pfn(addr & PAGE_MASK),
-                                   DIV_ROUND_UP(size, PAGE_SIZE),
-                                   paddr_to_pfn(addr & PAGE_MASK));
+        res = map_regions(d,
+                          paddr_to_pfn(addr & PAGE_MASK),
+                          DIV_ROUND_UP(size, PAGE_SIZE),
+                          paddr_to_pfn(addr & PAGE_MASK),
+                          MATTR_MEM, p2m_access_rw);
         if ( res )
         {
              panic(XENLOG_ERR "Unable to map ACPI region 0x%"PRIx64
@@ -1926,10 +1927,11 @@  static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     acpi_create_efi_mmap_table(d, &kinfo->mem, tbl_add);
 
     /* Map the EFI and ACPI tables to Dom0 */
-    rc = map_regions_rw_cache(d,
-                              paddr_to_pfn(d->arch.efi_acpi_gpa),
-                              PFN_UP(d->arch.efi_acpi_len),
-                              paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
+    rc = map_regions(d,
+                     paddr_to_pfn(d->arch.efi_acpi_gpa),
+                     PFN_UP(d->arch.efi_acpi_len),
+                     paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)),
+                     MATTR_MEM, p2m_access_rw);
     if ( rc != 0 )
     {
         printk(XENLOG_ERR "Unable to map EFI/ACPI table 0x%"PRIx64
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 0f1e1b1..a8538e5 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1217,30 +1217,34 @@  int p2m_populate_ram(struct domain *d,
                              d->arch.p2m.default_access);
 }
 
-int map_regions_rw_cache(struct domain *d,
-                         unsigned long start_gfn,
-                         unsigned long nr,
-                         unsigned long mfn)
+int map_regions(struct domain *d,
+                unsigned long start_gfn,
+                unsigned long nr,
+                unsigned long mfn,
+                unsigned int mattr,
+                p2m_access_t access)
 {
     return apply_p2m_changes(d, INSERT,
                              pfn_to_paddr(start_gfn),
                              pfn_to_paddr(start_gfn + nr),
                              pfn_to_paddr(mfn),
-                             MATTR_MEM, 0, p2m_mmio_direct,
-                             p2m_access_rw);
+                             mattr, 0, p2m_mmio_direct,
+                             access);
 }
 
-int unmap_regions_rw_cache(struct domain *d,
-                           unsigned long start_gfn,
-                           unsigned long nr,
-                           unsigned long mfn)
+int unmap_regions(struct domain *d,
+                  unsigned long start_gfn,
+                  unsigned long nr,
+                  unsigned long mfn,
+                  unsigned int mattr,
+                  p2m_access_t access)
 {
     return apply_p2m_changes(d, REMOVE,
                              pfn_to_paddr(start_gfn),
                              pfn_to_paddr(start_gfn + nr),
                              pfn_to_paddr(mfn),
-                             MATTR_MEM, 0, p2m_invalid,
-                             p2m_access_rw);
+                             mattr, 0, p2m_invalid,
+                             access);
 }
 
 int map_mmio_regions(struct domain *d,
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index d240d1e..92c425e 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -144,15 +144,19 @@  int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
 /* Setup p2m RAM mapping for domain d from start-end. */
 int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
 
-int map_regions_rw_cache(struct domain *d,
-                         unsigned long start_gfn,
-                         unsigned long nr_mfns,
-                         unsigned long mfn);
-
-int unmap_regions_rw_cache(struct domain *d,
-                           unsigned long start_gfn,
-                           unsigned long nr_mfns,
-                           unsigned long mfn);
+int map_regions(struct domain *d,
+                unsigned long start_gfn,
+                unsigned long nr,
+                unsigned long mfn,
+                unsigned int mattr,
+                p2m_access_t perms);
+
+int unmap_regions(struct domain *d,
+                  unsigned long start_gfn,
+                  unsigned long nr,
+                  unsigned long mfn,
+                  unsigned int mattr,
+                  p2m_access_t perms);
 
 int map_dev_mmio_region(struct domain *d,
                         unsigned long start_gfn,