[PATCH v4 0/5] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain

[PATCH v4 0/5] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain

[mjaggi]

From: Manish Jaggi <mjaggi@cavium.com>

This patch is split into 5 patches. First two add support for updating
host_its_list from ACPI MADT table.
The rest patches provide support to update the hardware domain MADT table
with ITS information.

Changes since v3
- Set GICV3_ITS_SIZE as 128K
- updated gicv2_get_hwdom_madt_size
- Removed offset from gicv3_its_make_hwdom_madt

Changes since v2:
- %s/u32/unsigned long
- %s/u64/paddr_t
- cleanup gicv3_its_make_hwdom_madt as per review comments
- remove gicv3_its_host_has_its() checks in patch 3
- removed gicv3_its_madt_generic_translator_size() 

Changes since v1:
- split patches into smaller ones
- removed translation_id

Manish Jaggi (5):
  ARM: ITS: Introduce common function add_to_host_its_list
  ARM: ITS: Populate host_its_list from ACPI MADT Table
  ARM: ITS: Deny hardware domain access to ITS
  ARM: Introduce get_hwdom_madt_size in gic_hw_operations
  ARM: ITS: Expose ITS in the MADT table

 xen/arch/arm/domain_build.c      |  7 +---
 xen/arch/arm/gic-v2.c            |  9 ++++
 xen/arch/arm/gic-v3-its.c        | 91 ++++++++++++++++++++++++++++++++++++----
 xen/arch/arm/gic-v3.c            | 25 +++++++++++
 xen/arch/arm/gic.c               | 12 ++++++
 xen/include/asm-arm/gic.h        |  3 ++
 xen/include/asm-arm/gic_v3_its.h | 27 ++++++++++++
 7 files changed, 159 insertions(+), 15 deletions(-)

-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 1/5] ARM: ITS: Introduce common function add_to_host_its_list

[mjaggi]

From: Manish Jaggi <mjaggi@cavium.com>

add_to_host_its_list will update the host_its_list. This common
function to be invoked from gicv3_its_dt_init and gic_v3_its_acpi_probe.

Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/gic-v3-its.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 2d36030..0610991 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -976,11 +976,29 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
     return res;
 }
 
+/* Common function for adding to host_its_list */
+static void add_to_host_its_list(paddr_t addr, paddr_t size,
+                                 const struct dt_device_node *node)
+{
+    struct host_its *its_data;
+
+    its_data = xzalloc(struct host_its);
+    if ( !its_data )
+        panic("GICv3: Cannot allocate memory for ITS frame");
+
+    its_data->addr = addr;
+    its_data->size = size;
+    its_data->dt_node = node;
+
+    printk("GICv3: Found ITS @0x%lx\n", addr);
+
+    list_add_tail(&its_data->entry, &host_its_list);
+}
+
 /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
 void gicv3_its_dt_init(const struct dt_device_node *node)
 {
     const struct dt_device_node *its = NULL;
-    struct host_its *its_data;
 
     /*
      * Check for ITS MSI subnodes. If any, add the ITS register
@@ -996,17 +1014,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
         if ( dt_device_get_address(its, 0, &addr, &size) )
             panic("GICv3: Cannot find a valid ITS frame address");
 
-        its_data = xzalloc(struct host_its);
-        if ( !its_data )
-            panic("GICv3: Cannot allocate memory for ITS frame");
-
-        its_data->addr = addr;
-        its_data->size = size;
-        its_data->dt_node = its;
-
-        printk("GICv3: Found ITS @0x%lx\n", addr);
-
-        list_add_tail(&its_data->entry, &host_its_list);
+        add_to_host_its_list(addr, size, its);
     }
 }
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table

[mjaggi]

From: Manish Jaggi <mjaggi@cavium.com>

Added gicv3_its_acpi_init to update host_its_list from MADT table.
For ACPI, host_its structure  stores dt_node as NULL.

Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
---
 xen/arch/arm/gic-v3-its.c        | 24 ++++++++++++++++++++++++
 xen/arch/arm/gic-v3.c            |  2 ++
 xen/include/asm-arm/gic_v3_its.h | 10 ++++++++++
 3 files changed, 36 insertions(+)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 0610991..0f662cf 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -18,6 +18,7 @@
  * along with this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <xen/acpi.h>
 #include <xen/lib.h>
 #include <xen/delay.h>
 #include <xen/libfdt/libfdt.h>
@@ -1018,6 +1019,29 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
     }
 }
 
+#ifdef CONFIG_ACPI
+static int gicv3_its_acpi_probe(struct acpi_subtable_header *header,
+                                const unsigned long end)
+{
+    struct acpi_madt_generic_translator *its;
+
+    its = (struct acpi_madt_generic_translator *)header;
+    if ( BAD_MADT_ENTRY(its, end) )
+        return -EINVAL;
+
+    add_to_host_its_list(its->base_address, GICV3_ITS_SIZE, NULL);
+
+    return 0;
+}
+
+void gicv3_its_acpi_init(void)
+{
+    /* Parse ITS information */
+    acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+                            gicv3_its_acpi_probe, 0);
+}
+#endif
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index f990eae..6f562f4 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1567,6 +1567,8 @@ static void __init gicv3_acpi_init(void)
 
     gicv3.rdist_stride = 0;
 
+    gicv3_its_acpi_init();
+
     /*
      * In ACPI, 0 is considered as the invalid address. However the rest
      * of the initialization rely on the invalid address to be
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index 1fac1c7..e1be33c 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -20,6 +20,7 @@
 #ifndef __ASM_ARM_ITS_H__
 #define __ASM_ARM_ITS_H__
 
+#define GICV3_ITS_SIZE                  SZ_128K
 #define GITS_CTLR                       0x000
 #define GITS_IIDR                       0x004
 #define GITS_TYPER                      0x008
@@ -135,6 +136,9 @@ extern struct list_head host_its_list;
 /* Parse the host DT and pick up all host ITSes. */
 void gicv3_its_dt_init(const struct dt_device_node *node);
 
+#ifdef CONFIG_ACPI
+void gicv3_its_acpi_init(void);
+#endif
 bool gicv3_its_host_has_its(void);
 
 unsigned int vgic_v3_its_count(const struct domain *d);
@@ -196,6 +200,12 @@ static inline void gicv3_its_dt_init(const struct dt_device_node *node)
 {
 }
 
+#ifdef CONFIG_ACPI
+static inline void gicv3_its_acpi_init(void)
+{
+}
+#endif
+
 static inline bool gicv3_its_host_has_its(void)
 {
     return false;
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 3/5] ARM: ITS: Deny hardware domain access to ITS

[mjaggi]

From: Manish Jaggi <mjaggi@cavium.com>

This patch extends the gicv3_iomem_deny_access functionality by adding
support for ITS region as well. Add function gicv3_its_deny_access.

Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
---
 xen/arch/arm/gic-v3-its.c        | 22 ++++++++++++++++++++++
 xen/arch/arm/gic-v3.c            |  3 +++
 xen/include/asm-arm/gic_v3_its.h |  9 +++++++++
 3 files changed, 34 insertions(+)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 0f662cf..8697e5b 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -21,6 +21,7 @@
 #include <xen/acpi.h>
 #include <xen/lib.h>
 #include <xen/delay.h>
+#include <xen/iocap.h>
 #include <xen/libfdt/libfdt.h>
 #include <xen/mm.h>
 #include <xen/rbtree.h>
@@ -905,6 +906,27 @@ struct pending_irq *gicv3_assign_guest_event(struct domain *d,
     return pirq;
 }
 
+int gicv3_its_deny_access(const struct domain *d)
+{
+    int rc = 0;
+    unsigned long mfn, nr;
+    const struct host_its *its_data;
+
+    list_for_each_entry( its_data, &host_its_list, entry )
+    {
+        mfn = paddr_to_pfn(its_data->addr);
+        nr = PFN_UP(GICV3_ITS_SIZE);
+        rc = iomem_deny_access(d, mfn, mfn + nr);
+        if ( rc )
+        {
+            printk( "iomem_deny_access failed for %lx:%lx \r\n", mfn, nr);
+            break;
+        }
+    }
+
+    return rc;
+}
+
 /*
  * Create the respective guest DT nodes from a list of host ITSes.
  * This copies the reg property, so the guest sees the ITS at the same address
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 6f562f4..b3d605d 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1308,6 +1308,9 @@ static int gicv3_iomem_deny_access(const struct domain *d)
     if ( rc )
         return rc;
 
+    if ( gicv3_its_deny_access(d) )
+        return rc;
+
     for ( i = 0; i < gicv3.rdist_count; i++ )
     {
         mfn = gicv3.rdist_regions[i].base >> PAGE_SHIFT;
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index e1be33c..31fca66 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -139,6 +139,10 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
 #ifdef CONFIG_ACPI
 void gicv3_its_acpi_init(void);
 #endif
+
+/* Deny iomem access for its */
+int gicv3_its_deny_access(const struct domain *d);
+
 bool gicv3_its_host_has_its(void);
 
 unsigned int vgic_v3_its_count(const struct domain *d);
@@ -206,6 +210,11 @@ static inline void gicv3_its_acpi_init(void)
 }
 #endif
 
+static inline int gicv3_its_deny_access(const struct domain *d)
+{
+    return 0;
+}
+
 static inline bool gicv3_its_host_has_its(void)
 {
     return false;
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations

[mjaggi]

From: Manish Jaggi <mjaggi@cavium.com>

estimate_acpi_efi_size needs to be updated to provide correct size of
hardware domains MADT, which now adds ITS information as well.

Introducing gic_get_hwdom_madt_size.

Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
---
 xen/arch/arm/domain_build.c |  7 +------
 xen/arch/arm/gic-v2.c       |  9 +++++++++
 xen/arch/arm/gic-v3.c       | 19 +++++++++++++++++++
 xen/arch/arm/gic.c          | 12 ++++++++++++
 xen/include/asm-arm/gic.h   |  3 +++
 5 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d6f9585..f17fcf1 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1808,12 +1808,7 @@ static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
     acpi_size = ROUNDUP(sizeof(struct acpi_table_fadt), 8);
     acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8);
 
-    madt_size = sizeof(struct acpi_table_madt)
-                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
-                + sizeof(struct acpi_madt_generic_distributor);
-    if ( d->arch.vgic.version == GIC_V3 )
-        madt_size += sizeof(struct acpi_madt_generic_redistributor)
-                     * d->arch.vgic.nr_regions;
+    madt_size = gic_get_hwdom_madt_size(d);
     acpi_size += ROUNDUP(madt_size, 8);
 
     addr = acpi_os_get_root_pointer();
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index cbe71a9..2868766 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1012,6 +1012,14 @@ static int gicv2_iomem_deny_access(const struct domain *d)
     return iomem_deny_access(d, mfn, mfn + nr);
 }
 
+static unsigned long gicv2_get_hwdom_madt_size(const struct domain *d)
+{
+    return sizeof(struct acpi_table_madt)
+                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
+                + sizeof(struct acpi_madt_generic_distributor);
+
+}
+
 #ifdef CONFIG_ACPI
 static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
 {
@@ -1248,6 +1256,7 @@ const static struct gic_hw_operations gicv2_ops = {
     .read_apr            = gicv2_read_apr,
     .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
     .make_hwdom_madt     = gicv2_make_hwdom_madt,
+    .get_hwdom_madt_size = gicv2_get_hwdom_madt_size,
     .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings,
     .iomem_deny_access   = gicv2_iomem_deny_access,
     .do_LPI              = gicv2_do_LPI,
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index b3d605d..6e8d580 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1406,6 +1406,19 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
     return table_len;
 }
 
+static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d)
+{
+    unsigned long size;
+
+    size  = sizeof(struct acpi_madt_generic_redistributor)
+                    * d->arch.vgic.nr_regions;
+
+    size  += vgic_v3_its_count(d)
+                    * sizeof(struct acpi_madt_generic_translator);
+
+    return size;
+}
+
 static int __init
 gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
                         const unsigned long end)
@@ -1597,6 +1610,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
 {
     return 0;
 }
+
+static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d)
+{
+    return 0;
+}
 #endif
 
 /* Set up the GIC */
@@ -1698,6 +1716,7 @@ static const struct gic_hw_operations gicv3_ops = {
     .secondary_init      = gicv3_secondary_cpu_init,
     .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
     .make_hwdom_madt     = gicv3_make_hwdom_madt,
+    .get_hwdom_madt_size = gicv3_get_hwdom_madt_size,
     .iomem_deny_access   = gicv3_iomem_deny_access,
     .do_LPI              = gicv3_do_LPI,
 };
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 6c803bf..f3c1f0b 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -851,6 +851,18 @@ int gic_make_hwdom_madt(const struct domain *d, u32 offset)
     return gic_hw_ops->make_hwdom_madt(d, offset);
 }
 
+unsigned long gic_get_hwdom_madt_size(const struct domain *d)
+{
+    unsigned long madt_size;
+
+    madt_size = sizeof(struct acpi_table_madt)
+                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
+                + sizeof(struct acpi_madt_generic_distributor)
+                + gic_hw_ops->get_hwdom_madt_size(d);
+
+    return madt_size;
+}
+
 int gic_iomem_deny_access(const struct domain *d)
 {
     return gic_hw_ops->iomem_deny_access(d);
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 6203dc5..3acdd6d 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -365,6 +365,8 @@ struct gic_hw_operations {
     int (*make_hwdom_madt)(const struct domain *d, u32 offset);
     /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */
     int (*map_hwdom_extra_mappings)(struct domain *d);
+    /* Query the size of hardware domain madt table */
+    unsigned long (*get_hwdom_madt_size)(const struct domain *d);
     /* Deny access to GIC regions */
     int (*iomem_deny_access)(const struct domain *d);
     /* Handle LPIs, which require special handling */
@@ -376,6 +378,7 @@ int gic_make_hwdom_dt_node(const struct domain *d,
                            const struct dt_device_node *gic,
                            void *fdt);
 int gic_make_hwdom_madt(const struct domain *d, u32 offset);
+unsigned long gic_get_hwdom_madt_size(const struct domain *d);
 int gic_map_hwdom_extra_mappings(struct domain *d);
 int gic_iomem_deny_access(const struct domain *d);
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 5/5] ARM: ITS: Expose ITS in the MADT table

[mjaggi]

From: Manish Jaggi <mjaggi@cavium.com>

Add gicv3_its_make_hwdom_madt to update hwdom MADT ITS information.

Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
---
 xen/arch/arm/gic-v3-its.c        | 19 +++++++++++++++++++
 xen/arch/arm/gic-v3.c            |  1 +
 xen/include/asm-arm/gic_v3_its.h |  8 ++++++++
 3 files changed, 28 insertions(+)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 8697e5b..e3e7e92 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -1062,6 +1062,25 @@ void gicv3_its_acpi_init(void)
     acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
                             gicv3_its_acpi_probe, 0);
 }
+
+unsigned long gicv3_its_make_hwdom_madt(const struct domain *d, void *base_ptr)
+{
+    unsigned long i = 0;
+    void *fw_its;
+    struct acpi_madt_generic_translator *hwdom_its;
+
+    hwdom_its = base_ptr;
+
+    for ( i = 0; i < vgic_v3_its_count(d); i++ )
+    {
+        fw_its = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+                                           i);
+        memcpy(hwdom_its, fw_its, sizeof(struct acpi_madt_generic_translator));
+        hwdom_its++;
+    }
+
+    return sizeof(struct acpi_madt_generic_translator) * vgic_v3_its_count(d);
+}
 #endif
 
 /*
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 6e8d580..d29eea6 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1403,6 +1403,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
         table_len += size;
     }
 
+    table_len += gicv3_its_make_hwdom_madt(d, base_ptr + table_len);
     return table_len;
 }
 
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index 31fca66..fc37776 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -138,6 +138,8 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
 
 #ifdef CONFIG_ACPI
 void gicv3_its_acpi_init(void);
+unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
+                                        void *base_ptr);
 #endif
 
 /* Deny iomem access for its */
@@ -208,6 +210,12 @@ static inline void gicv3_its_dt_init(const struct dt_device_node *node)
 static inline void gicv3_its_acpi_init(void)
 {
 }
+
+static inline unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
+                                                      void *base_ptr)
+{
+    return 0;
+}
 #endif
 
 static inline int gicv3_its_deny_access(const struct domain *d)
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table

[Julien Grall]

Hi Manish,

On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <mjaggi@cavium.com>
> 
> Added gicv3_its_acpi_init to update host_its_list from MADT table.
> For ACPI, host_its structure  stores dt_node as NULL.
> 
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
> ---
>   xen/arch/arm/gic-v3-its.c        | 24 ++++++++++++++++++++++++
>   xen/arch/arm/gic-v3.c            |  2 ++
>   xen/include/asm-arm/gic_v3_its.h | 10 ++++++++++
>   3 files changed, 36 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 0610991..0f662cf 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -18,6 +18,7 @@
>    * along with this program; If not, see <http://www.gnu.org/licenses/>.
>    */
>   
> +#include <xen/acpi.h>
>   #include <xen/lib.h>
>   #include <xen/delay.h>
>   #include <xen/libfdt/libfdt.h>
> @@ -1018,6 +1019,29 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
>       }
>   }
>   
> +#ifdef CONFIG_ACPI
> +static int gicv3_its_acpi_probe(struct acpi_subtable_header *header,
> +                                const unsigned long end)
> +{
> +    struct acpi_madt_generic_translator *its;
> +
> +    its = (struct acpi_madt_generic_translator *)header;
> +    if ( BAD_MADT_ENTRY(its, end) )
> +        return -EINVAL;
> +
> +    add_to_host_its_list(its->base_address, GICV3_ITS_SIZE, NULL);

After the comment from Andre, I was expecting some rework to avoid store 
the size of the ITS in host_its. So what's the plan for that?

> +
> +    return 0;
> +}
> +
> +void gicv3_its_acpi_init(void)
> +{
> +    /* Parse ITS information */
> +    acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> +                            gicv3_its_acpi_probe, 0);

The indentation still looks wrong here.

> +}
> +#endif
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index f990eae..6f562f4 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1567,6 +1567,8 @@ static void __init gicv3_acpi_init(void)
>   
>       gicv3.rdist_stride = 0;
>   
> +    gicv3_its_acpi_init();
> +
>       /*
>        * In ACPI, 0 is considered as the invalid address. However the rest
>        * of the initialization rely on the invalid address to be
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index 1fac1c7..e1be33c 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -20,6 +20,7 @@
>   #ifndef __ASM_ARM_ITS_H__
>   #define __ASM_ARM_ITS_H__
>   
> +#define GICV3_ITS_SIZE                  SZ_128K

A less random place for this is close to the ITS_DOORBELL_OFFSET definition.

>   #define GITS_CTLR                       0x000
>   #define GITS_IIDR                       0x004
>   #define GITS_TYPER                      0x008
> @@ -135,6 +136,9 @@ extern struct list_head host_its_list;
>   /* Parse the host DT and pick up all host ITSes. */
>   void gicv3_its_dt_init(const struct dt_device_node *node);
>   
> +#ifdef CONFIG_ACPI
> +void gicv3_its_acpi_init(void);
> +#endif
>   bool gicv3_its_host_has_its(void);
>   
>   unsigned int vgic_v3_its_count(const struct domain *d);
> @@ -196,6 +200,12 @@ static inline void gicv3_its_dt_init(const struct dt_device_node *node)
>   {
>   }
>   
> +#ifdef CONFIG_ACPI
> +static inline void gicv3_its_acpi_init(void)
> +{
> +}
> +#endif
> +
>   static inline bool gicv3_its_host_has_its(void)
>   {
>       return false;
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 3/5] ARM: ITS: Deny hardware domain access to ITS

[Julien Grall]

Hi Manish,

On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <mjaggi@cavium.com>
> 
> This patch extends the gicv3_iomem_deny_access functionality by adding
> support for ITS region as well. Add function gicv3_its_deny_access.
> 
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations

[Julien Grall]

Hi,

On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <mjaggi@cavium.com>
> 
> estimate_acpi_efi_size needs to be updated to provide correct size of
> hardware domains MADT, which now adds ITS information as well.
> 
> Introducing gic_get_hwdom_madt_size.

I think the commit title is misleading, the main purpose of this patch 
is updating the formula to compute the MADT size for GICv3. Not 
introducing the callbacks.

But likely, you want two patches here:
	- Patch #1 adding the callbacks
	- Patch #2 updating the formula for GICv3

For this time, I would be ok to have only one patch providing the commit 
message is updated.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 5/5] ARM: ITS: Expose ITS in the MADT table

[Julien Grall]

Hi Manish,

On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <mjaggi@cavium.com>
> 
> Add gicv3_its_make_hwdom_madt to update hwdom MADT ITS information.
> 
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
> ---
>   xen/arch/arm/gic-v3-its.c        | 19 +++++++++++++++++++
>   xen/arch/arm/gic-v3.c            |  1 +
>   xen/include/asm-arm/gic_v3_its.h |  8 ++++++++
>   3 files changed, 28 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 8697e5b..e3e7e92 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -1062,6 +1062,25 @@ void gicv3_its_acpi_init(void)
>       acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
>                               gicv3_its_acpi_probe, 0);
>   }
> +
> +unsigned long gicv3_its_make_hwdom_madt(const struct domain *d, void *base_ptr)
> +{
> +    unsigned long i = 0;
> +    void *fw_its;
> +    struct acpi_madt_generic_translator *hwdom_its;
> +
> +    hwdom_its = base_ptr;
> +
> +    for ( i = 0; i < vgic_v3_its_count(d); i++ )
> +    {
> +        fw_its = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> +                                           i);
> +        memcpy(hwdom_its, fw_its, sizeof(struct acpi_madt_generic_translator));
> +        hwdom_its++;
> +    }
> +
> +    return sizeof(struct acpi_madt_generic_translator) * vgic_v3_its_count(d);
> +}
>   #endif
>   
>   /*
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 6e8d580..d29eea6 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1403,6 +1403,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
>           table_len += size;
>       }
>   
> +    table_len += gicv3_its_make_hwdom_madt(d, base_ptr + table_len);

Newline here please.

I will leave Andre to comment on this patch as he suggested the rework.

Cheers,

>       return table_len;
>   }
>   
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index 31fca66..fc37776 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -138,6 +138,8 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
>   
>   #ifdef CONFIG_ACPI
>   void gicv3_its_acpi_init(void);
> +unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
> +                                        void *base_ptr);
>   #endif
>   
>   /* Deny iomem access for its */
> @@ -208,6 +210,12 @@ static inline void gicv3_its_dt_init(const struct dt_device_node *node)
>   static inline void gicv3_its_acpi_init(void)
>   {
>   }
> +
> +static inline unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
> +                                                      void *base_ptr)
> +{
> +    return 0;
> +}
>   #endif
>   
>   static inline int gicv3_its_deny_access(const struct domain *d)
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table

[Manish Jaggi]

Hello Julien,

On 10/3/2017 7:17 PM, Julien Grall wrote:
> Hi Manish,
>
> On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
>> From: Manish Jaggi <mjaggi@cavium.com>
>>
>> Added gicv3_its_acpi_init to update host_its_list from MADT table.
>> For ACPI, host_its structure  stores dt_node as NULL.
>>
>> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
>> ---
>>   xen/arch/arm/gic-v3-its.c        | 24 ++++++++++++++++++++++++
>>   xen/arch/arm/gic-v3.c            |  2 ++
>>   xen/include/asm-arm/gic_v3_its.h | 10 ++++++++++
>>   3 files changed, 36 insertions(+)
>>
>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> index 0610991..0f662cf 100644
>> --- a/xen/arch/arm/gic-v3-its.c
>> +++ b/xen/arch/arm/gic-v3-its.c
>> @@ -18,6 +18,7 @@
>>    * along with this program; If not, see 
>> <http://www.gnu.org/licenses/>.
>>    */
>>   +#include <xen/acpi.h>
>>   #include <xen/lib.h>
>>   #include <xen/delay.h>
>>   #include <xen/libfdt/libfdt.h>
>> @@ -1018,6 +1019,29 @@ void gicv3_its_dt_init(const struct 
>> dt_device_node *node)
>>       }
>>   }
>>   +#ifdef CONFIG_ACPI
>> +static int gicv3_its_acpi_probe(struct acpi_subtable_header *header,
>> +                                const unsigned long end)
>> +{
>> +    struct acpi_madt_generic_translator *its;
>> +
>> +    its = (struct acpi_madt_generic_translator *)header;
>> +    if ( BAD_MADT_ENTRY(its, end) )
>> +        return -EINVAL;
>> +
>> +    add_to_host_its_list(its->base_address, GICV3_ITS_SIZE, NULL);
>
> After the comment from Andre, I was expecting some rework to avoid 
> store the size of the ITS in host_its. So what's the plan for that?
GICV3_ITS_SIZE  is now 128K (prev 64k, see below), same as what used in 
linux code, I think andre mentioned that need to add additional 64K.
>
>> +
>> +    return 0;
>> +}
>> +
>> +void gicv3_its_acpi_init(void)
>> +{
>> +    /* Parse ITS information */
>> +    acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
>> +                            gicv3_its_acpi_probe, 0);
>
> The indentation still looks wrong here.
ah.. ok.
>
>> +}
>> +#endif
>> +
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index f990eae..6f562f4 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -1567,6 +1567,8 @@ static void __init gicv3_acpi_init(void)
>>         gicv3.rdist_stride = 0;
>>   +    gicv3_its_acpi_init();
>> +
>>       /*
>>        * In ACPI, 0 is considered as the invalid address. However the 
>> rest
>>        * of the initialization rely on the invalid address to be
>> diff --git a/xen/include/asm-arm/gic_v3_its.h 
>> b/xen/include/asm-arm/gic_v3_its.h
>> index 1fac1c7..e1be33c 100644
>> --- a/xen/include/asm-arm/gic_v3_its.h
>> +++ b/xen/include/asm-arm/gic_v3_its.h
>> @@ -20,6 +20,7 @@
>>   #ifndef __ASM_ARM_ITS_H__
>>   #define __ASM_ARM_ITS_H__
>>   +#define GICV3_ITS_SIZE                  SZ_128K
>
> A less random place for this is close to the ITS_DOORBELL_OFFSET 
> definition.
ok will do :)
>
>>   #define GITS_CTLR 0x000
>>   #define GITS_IIDR                       0x004
>>   #define GITS_TYPER                      0x008
>> @@ -135,6 +136,9 @@ extern struct list_head host_its_list;
>>   /* Parse the host DT and pick up all host ITSes. */
>>   void gicv3_its_dt_init(const struct dt_device_node *node);
>>   +#ifdef CONFIG_ACPI
>> +void gicv3_its_acpi_init(void);
>> +#endif
>>   bool gicv3_its_host_has_its(void);
>>     unsigned int vgic_v3_its_count(const struct domain *d);
>> @@ -196,6 +200,12 @@ static inline void gicv3_its_dt_init(const 
>> struct dt_device_node *node)
>>   {
>>   }
>>   +#ifdef CONFIG_ACPI
>> +static inline void gicv3_its_acpi_init(void)
>> +{
>> +}
>> +#endif
>> +
>>   static inline bool gicv3_its_host_has_its(void)
>>   {
>>       return false;
>>
>
> Cheers,
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations

[Manish Jaggi]

Hi

On 10/3/2017 8:01 PM, Julien Grall wrote:
> Hi,
>
> On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
>> From: Manish Jaggi <mjaggi@cavium.com>
>>
>> estimate_acpi_efi_size needs to be updated to provide correct size of
>> hardware domains MADT, which now adds ITS information as well.
>>
>> Introducing gic_get_hwdom_madt_size.
>
> I think the commit title is misleading, the main purpose of this patch 
> is updating the formula to compute the MADT size for GICv3. Not 
> introducing the callbacks.
>
> But likely, you want two patches here:
>     - Patch #1 adding the callbacks
>     - Patch #2 updating the formula for GICv3
>
> For this time, I would be ok to have only one patch providing the 
> commit message is updated.
>
ok, will update..
> Cheers,
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 5/5] ARM: ITS: Expose ITS in the MADT table

[Manish Jaggi]

Hi Andre,

On 10/3/2017 8:03 PM, Julien Grall wrote:
> Hi Manish,
>
> On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
>> From: Manish Jaggi <mjaggi@cavium.com>
>>
>> Add gicv3_its_make_hwdom_madt to update hwdom MADT ITS information.
>>
>> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
>> ---
>>   xen/arch/arm/gic-v3-its.c        | 19 +++++++++++++++++++
>>   xen/arch/arm/gic-v3.c            |  1 +
>>   xen/include/asm-arm/gic_v3_its.h |  8 ++++++++
>>   3 files changed, 28 insertions(+)
>>
>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> index 8697e5b..e3e7e92 100644
>> --- a/xen/arch/arm/gic-v3-its.c
>> +++ b/xen/arch/arm/gic-v3-its.c
>> @@ -1062,6 +1062,25 @@ void gicv3_its_acpi_init(void)
>>       acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
>>                               gicv3_its_acpi_probe, 0);
>>   }
>> +
>> +unsigned long gicv3_its_make_hwdom_madt(const struct domain *d, void 
>> *base_ptr)
>> +{
>> +    unsigned long i = 0;
>> +    void *fw_its;
>> +    struct acpi_madt_generic_translator *hwdom_its;
>> +
>> +    hwdom_its = base_ptr;
>> +
>> +    for ( i = 0; i < vgic_v3_its_count(d); i++ )
>> +    {
>> +        fw_its = 
>> acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
>> +                                           i);
>> +        memcpy(hwdom_its, fw_its, sizeof(struct 
>> acpi_madt_generic_translator));
>> +        hwdom_its++;
>> +    }
>> +
>> +    return sizeof(struct acpi_madt_generic_translator) * 
>> vgic_v3_its_count(d);
>> +}
>>   #endif
>>     /*
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index 6e8d580..d29eea6 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -1403,6 +1403,7 @@ static int gicv3_make_hwdom_madt(const struct 
>> domain *d, u32 offset)
>>           table_len += size;
>>       }
>>   +    table_len += gicv3_its_make_hwdom_madt(d, base_ptr + table_len);
>
> Newline here please.
>
> I will leave Andre to comment on this patch as he suggested the rework.
Could you please provide comments on this patch so that I can send an 
updated v5.
>
> Cheers,
>
>>       return table_len;
>>   }
>>   diff --git a/xen/include/asm-arm/gic_v3_its.h 
>> b/xen/include/asm-arm/gic_v3_its.h
>> index 31fca66..fc37776 100644
>> --- a/xen/include/asm-arm/gic_v3_its.h
>> +++ b/xen/include/asm-arm/gic_v3_its.h
>> @@ -138,6 +138,8 @@ void gicv3_its_dt_init(const struct 
>> dt_device_node *node);
>>     #ifdef CONFIG_ACPI
>>   void gicv3_its_acpi_init(void);
>> +unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
>> +                                        void *base_ptr);
>>   #endif
>>     /* Deny iomem access for its */
>> @@ -208,6 +210,12 @@ static inline void gicv3_its_dt_init(const 
>> struct dt_device_node *node)
>>   static inline void gicv3_its_acpi_init(void)
>>   {
>>   }
>> +
>> +static inline unsigned long gicv3_its_make_hwdom_madt(const struct 
>> domain *d,
>> +                                                      void *base_ptr)
>> +{
>> +    return 0;
>> +}
>>   #endif
>>     static inline int gicv3_its_deny_access(const struct domain *d)
>>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table

[Andre Przywara]

Hi,

On 04/10/17 06:29, Manish Jaggi wrote:
> Hello Julien,
> 
> On 10/3/2017 7:17 PM, Julien Grall wrote:
>> Hi Manish,
>>
>> On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
>>> From: Manish Jaggi <mjaggi@cavium.com>
>>>
>>> Added gicv3_its_acpi_init to update host_its_list from MADT table.
>>> For ACPI, host_its structure  stores dt_node as NULL.
>>>
>>> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
>>> ---
>>>   xen/arch/arm/gic-v3-its.c        | 24 ++++++++++++++++++++++++
>>>   xen/arch/arm/gic-v3.c            |  2 ++
>>>   xen/include/asm-arm/gic_v3_its.h | 10 ++++++++++
>>>   3 files changed, 36 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>>> index 0610991..0f662cf 100644
>>> --- a/xen/arch/arm/gic-v3-its.c
>>> +++ b/xen/arch/arm/gic-v3-its.c
>>> @@ -18,6 +18,7 @@
>>>    * along with this program; If not, see
>>> <http://www.gnu.org/licenses/>.
>>>    */
>>>   +#include <xen/acpi.h>
>>>   #include <xen/lib.h>
>>>   #include <xen/delay.h>
>>>   #include <xen/libfdt/libfdt.h>
>>> @@ -1018,6 +1019,29 @@ void gicv3_its_dt_init(const struct
>>> dt_device_node *node)
>>>       }
>>>   }
>>>   +#ifdef CONFIG_ACPI
>>> +static int gicv3_its_acpi_probe(struct acpi_subtable_header *header,
>>> +                                const unsigned long end)
>>> +{
>>> +    struct acpi_madt_generic_translator *its;
>>> +
>>> +    its = (struct acpi_madt_generic_translator *)header;
>>> +    if ( BAD_MADT_ENTRY(its, end) )
>>> +        return -EINVAL;
>>> +
>>> +    add_to_host_its_list(its->base_address, GICV3_ITS_SIZE, NULL);
>>
>> After the comment from Andre, I was expecting some rework to avoid
>> store the size of the ITS in host_its. So what's the plan for that?
> GICV3_ITS_SIZE  is now 128K (prev 64k, see below), same as what used in
> linux code, I think andre mentioned that need to add additional 64K.

That was one thing, but I was wondering about why we would need to store
that value as a *variable* in struct host_its when it is actually an
architecture defined constant. But as it was there before and it seems
cleaner to use the DT provided size, it could stay as well. We might fix
that later on.

>>
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +void gicv3_its_acpi_init(void)
>>> +{
>>> +    /* Parse ITS information */
>>> +    acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
>>> +                            gicv3_its_acpi_probe, 0);
>>
>> The indentation still looks wrong here.
> ah.. ok.

So ignoring that "size" thing above and assuming this w/s issue fixed:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

>>
>>> +}
>>> +#endif
>>> +
>>>   /*
>>>    * Local variables:
>>>    * mode: C
>>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>>> index f990eae..6f562f4 100644
>>> --- a/xen/arch/arm/gic-v3.c
>>> +++ b/xen/arch/arm/gic-v3.c
>>> @@ -1567,6 +1567,8 @@ static void __init gicv3_acpi_init(void)
>>>         gicv3.rdist_stride = 0;
>>>   +    gicv3_its_acpi_init();
>>> +
>>>       /*
>>>        * In ACPI, 0 is considered as the invalid address. However the
>>> rest
>>>        * of the initialization rely on the invalid address to be
>>> diff --git a/xen/include/asm-arm/gic_v3_its.h
>>> b/xen/include/asm-arm/gic_v3_its.h
>>> index 1fac1c7..e1be33c 100644
>>> --- a/xen/include/asm-arm/gic_v3_its.h
>>> +++ b/xen/include/asm-arm/gic_v3_its.h
>>> @@ -20,6 +20,7 @@
>>>   #ifndef __ASM_ARM_ITS_H__
>>>   #define __ASM_ARM_ITS_H__
>>>   +#define GICV3_ITS_SIZE                  SZ_128K
>>
>> A less random place for this is close to the ITS_DOORBELL_OFFSET
>> definition.
> ok will do :)
>>
>>>   #define GITS_CTLR 0x000
>>>   #define GITS_IIDR                       0x004
>>>   #define GITS_TYPER                      0x008
>>> @@ -135,6 +136,9 @@ extern struct list_head host_its_list;
>>>   /* Parse the host DT and pick up all host ITSes. */
>>>   void gicv3_its_dt_init(const struct dt_device_node *node);
>>>   +#ifdef CONFIG_ACPI
>>> +void gicv3_its_acpi_init(void);
>>> +#endif
>>>   bool gicv3_its_host_has_its(void);
>>>     unsigned int vgic_v3_its_count(const struct domain *d);
>>> @@ -196,6 +200,12 @@ static inline void gicv3_its_dt_init(const
>>> struct dt_device_node *node)
>>>   {
>>>   }
>>>   +#ifdef CONFIG_ACPI
>>> +static inline void gicv3_its_acpi_init(void)
>>> +{
>>> +}
>>> +#endif
>>> +
>>>   static inline bool gicv3_its_host_has_its(void)
>>>   {
>>>       return false;
>>>
>>
>> Cheers,
>>
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 3/5] ARM: ITS: Deny hardware domain access to ITS

[Andre Przywara]

Hi Manish,

On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <mjaggi@cavium.com>
> 
> This patch extends the gicv3_iomem_deny_access functionality by adding
> support for ITS region as well. Add function gicv3_its_deny_access.
> 
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks,
Andre.

> ---
>  xen/arch/arm/gic-v3-its.c        | 22 ++++++++++++++++++++++
>  xen/arch/arm/gic-v3.c            |  3 +++
>  xen/include/asm-arm/gic_v3_its.h |  9 +++++++++
>  3 files changed, 34 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 0f662cf..8697e5b 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -21,6 +21,7 @@
>  #include <xen/acpi.h>
>  #include <xen/lib.h>
>  #include <xen/delay.h>
> +#include <xen/iocap.h>
>  #include <xen/libfdt/libfdt.h>
>  #include <xen/mm.h>
>  #include <xen/rbtree.h>
> @@ -905,6 +906,27 @@ struct pending_irq *gicv3_assign_guest_event(struct domain *d,
>      return pirq;
>  }
>  
> +int gicv3_its_deny_access(const struct domain *d)
> +{
> +    int rc = 0;
> +    unsigned long mfn, nr;
> +    const struct host_its *its_data;
> +
> +    list_for_each_entry( its_data, &host_its_list, entry )
> +    {
> +        mfn = paddr_to_pfn(its_data->addr);
> +        nr = PFN_UP(GICV3_ITS_SIZE);
> +        rc = iomem_deny_access(d, mfn, mfn + nr);
> +        if ( rc )
> +        {
> +            printk( "iomem_deny_access failed for %lx:%lx \r\n", mfn, nr);
> +            break;
> +        }
> +    }
> +
> +    return rc;
> +}
> +
>  /*
>   * Create the respective guest DT nodes from a list of host ITSes.
>   * This copies the reg property, so the guest sees the ITS at the same address
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 6f562f4..b3d605d 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1308,6 +1308,9 @@ static int gicv3_iomem_deny_access(const struct domain *d)
>      if ( rc )
>          return rc;
>  
> +    if ( gicv3_its_deny_access(d) )
> +        return rc;
> +
>      for ( i = 0; i < gicv3.rdist_count; i++ )
>      {
>          mfn = gicv3.rdist_regions[i].base >> PAGE_SHIFT;
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index e1be33c..31fca66 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -139,6 +139,10 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
>  #ifdef CONFIG_ACPI
>  void gicv3_its_acpi_init(void);
>  #endif
> +
> +/* Deny iomem access for its */
> +int gicv3_its_deny_access(const struct domain *d);
> +
>  bool gicv3_its_host_has_its(void);
>  
>  unsigned int vgic_v3_its_count(const struct domain *d);
> @@ -206,6 +210,11 @@ static inline void gicv3_its_acpi_init(void)
>  }
>  #endif
>  
> +static inline int gicv3_its_deny_access(const struct domain *d)
> +{
> +    return 0;
> +}
> +
>  static inline bool gicv3_its_host_has_its(void)
>  {
>      return false;
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations

[Andre Przywara]

Hi,

On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <mjaggi@cavium.com>
> 
> estimate_acpi_efi_size needs to be updated to provide correct size of
> hardware domains MADT, which now adds ITS information as well.
> 
> Introducing gic_get_hwdom_madt_size.
> 
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
> ---
>  xen/arch/arm/domain_build.c |  7 +------
>  xen/arch/arm/gic-v2.c       |  9 +++++++++
>  xen/arch/arm/gic-v3.c       | 19 +++++++++++++++++++
>  xen/arch/arm/gic.c          | 12 ++++++++++++
>  xen/include/asm-arm/gic.h   |  3 +++
>  5 files changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index d6f9585..f17fcf1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1808,12 +1808,7 @@ static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
>      acpi_size = ROUNDUP(sizeof(struct acpi_table_fadt), 8);
>      acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8);
>  
> -    madt_size = sizeof(struct acpi_table_madt)
> -                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
> -                + sizeof(struct acpi_madt_generic_distributor);
> -    if ( d->arch.vgic.version == GIC_V3 )
> -        madt_size += sizeof(struct acpi_madt_generic_redistributor)
> -                     * d->arch.vgic.nr_regions;
> +    madt_size = gic_get_hwdom_madt_size(d);
>      acpi_size += ROUNDUP(madt_size, 8);
>  
>      addr = acpi_os_get_root_pointer();
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index cbe71a9..2868766 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -1012,6 +1012,14 @@ static int gicv2_iomem_deny_access(const struct domain *d)
>      return iomem_deny_access(d, mfn, mfn + nr);
>  }
>  
> +static unsigned long gicv2_get_hwdom_madt_size(const struct domain *d)
> +{
> +    return sizeof(struct acpi_table_madt)
> +                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
> +                + sizeof(struct acpi_madt_generic_distributor);
> +
> +}
> +
>  #ifdef CONFIG_ACPI
>  static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
>  {
> @@ -1248,6 +1256,7 @@ const static struct gic_hw_operations gicv2_ops = {
>      .read_apr            = gicv2_read_apr,
>      .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
>      .make_hwdom_madt     = gicv2_make_hwdom_madt,
> +    .get_hwdom_madt_size = gicv2_get_hwdom_madt_size,
>      .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings,
>      .iomem_deny_access   = gicv2_iomem_deny_access,
>      .do_LPI              = gicv2_do_LPI,
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index b3d605d..6e8d580 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1406,6 +1406,19 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
>      return table_len;
>  }
>  
> +static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d)
> +{
> +    unsigned long size;
> +
> +    size  = sizeof(struct acpi_madt_generic_redistributor)
> +                    * d->arch.vgic.nr_regions;
> +
> +    size  += vgic_v3_its_count(d)
> +                    * sizeof(struct acpi_madt_generic_translator);
> +
> +    return size;
> +}
> +
>  static int __init
>  gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
>                          const unsigned long end)
> @@ -1597,6 +1610,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
>  {
>      return 0;
>  }
> +
> +static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d)
> +{
> +    return 0;
> +}
>  #endif
>  
>  /* Set up the GIC */
> @@ -1698,6 +1716,7 @@ static const struct gic_hw_operations gicv3_ops = {
>      .secondary_init      = gicv3_secondary_cpu_init,
>      .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
>      .make_hwdom_madt     = gicv3_make_hwdom_madt,
> +    .get_hwdom_madt_size = gicv3_get_hwdom_madt_size,
>      .iomem_deny_access   = gicv3_iomem_deny_access,
>      .do_LPI              = gicv3_do_LPI,
>  };
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 6c803bf..f3c1f0b 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -851,6 +851,18 @@ int gic_make_hwdom_madt(const struct domain *d, u32 offset)
>      return gic_hw_ops->make_hwdom_madt(d, offset);
>  }
>  
> +unsigned long gic_get_hwdom_madt_size(const struct domain *d)
> +{
> +    unsigned long madt_size;
> +
> +    madt_size = sizeof(struct acpi_table_madt)
> +                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
> +                + sizeof(struct acpi_madt_generic_distributor)
> +                + gic_hw_ops->get_hwdom_madt_size(d);

But this is now doubled for a GICv2? As you already do that calculation
in the GICv2 callback?
So I suggest you drop that *there* and rename the function member to
get_hwdom_extra_madt_size() (or so).
So in the GICv2 version you can now return 0. Keep the GICv3 version as
it is.

That solution should be both readable and avoiding code duplication.

Cheers,
Andre.

> +
> +    return madt_size;
> +}
> +
>  int gic_iomem_deny_access(const struct domain *d)
>  {
>      return gic_hw_ops->iomem_deny_access(d);
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 6203dc5..3acdd6d 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -365,6 +365,8 @@ struct gic_hw_operations {
>      int (*make_hwdom_madt)(const struct domain *d, u32 offset);
>      /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */
>      int (*map_hwdom_extra_mappings)(struct domain *d);
> +    /* Query the size of hardware domain madt table */
> +    unsigned long (*get_hwdom_madt_size)(const struct domain *d);
>      /* Deny access to GIC regions */
>      int (*iomem_deny_access)(const struct domain *d);
>      /* Handle LPIs, which require special handling */
> @@ -376,6 +378,7 @@ int gic_make_hwdom_dt_node(const struct domain *d,
>                             const struct dt_device_node *gic,
>                             void *fdt);
>  int gic_make_hwdom_madt(const struct domain *d, u32 offset);
> +unsigned long gic_get_hwdom_madt_size(const struct domain *d);
>  int gic_map_hwdom_extra_mappings(struct domain *d);
>  int gic_iomem_deny_access(const struct domain *d);
>  
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 5/5] ARM: ITS: Expose ITS in the MADT table

[Andre Przywara]

Hi Manish,

On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <mjaggi@cavium.com>
> 
> Add gicv3_its_make_hwdom_madt to update hwdom MADT ITS information.

Thanks for the rework, that looks much better now!

> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> ---
>  xen/arch/arm/gic-v3-its.c        | 19 +++++++++++++++++++
>  xen/arch/arm/gic-v3.c            |  1 +
>  xen/include/asm-arm/gic_v3_its.h |  8 ++++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 8697e5b..e3e7e92 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -1062,6 +1062,25 @@ void gicv3_its_acpi_init(void)
>      acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
>                              gicv3_its_acpi_probe, 0);
>  }
> +
> +unsigned long gicv3_its_make_hwdom_madt(const struct domain *d, void *base_ptr)
> +{
> +    unsigned long i = 0;
> +    void *fw_its;
> +    struct acpi_madt_generic_translator *hwdom_its;
> +
> +    hwdom_its = base_ptr;
> +
> +    for ( i = 0; i < vgic_v3_its_count(d); i++ )
> +    {
> +        fw_its = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> +                                           i);
> +        memcpy(hwdom_its, fw_its, sizeof(struct acpi_madt_generic_translator));
> +        hwdom_its++;
> +    }
> +
> +    return sizeof(struct acpi_madt_generic_translator) * vgic_v3_its_count(d);
> +}
>  #endif
>  
>  /*
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 6e8d580..d29eea6 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1403,6 +1403,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
>          table_len += size;
>      }
>  
> +    table_len += gicv3_its_make_hwdom_madt(d, base_ptr + table_len);
>      return table_len;
>  }
>  
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index 31fca66..fc37776 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -138,6 +138,8 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
>  
>  #ifdef CONFIG_ACPI
>  void gicv3_its_acpi_init(void);
> +unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
> +                                        void *base_ptr);
>  #endif
>  
>  /* Deny iomem access for its */
> @@ -208,6 +210,12 @@ static inline void gicv3_its_dt_init(const struct dt_device_node *node)
>  static inline void gicv3_its_acpi_init(void)
>  {
>  }
> +
> +static inline unsigned long gicv3_its_make_hwdom_madt(const struct domain *d,
> +                                                      void *base_ptr)
> +{
> +    return 0;
> +}
>  #endif
>  
>  static inline int gicv3_its_deny_access(const struct domain *d)
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel