* [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list
2017-08-13 21:30 [PATCH v2 0/5] [resend] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain mjaggi
@ 2017-08-13 21:30 ` mjaggi
2017-08-22 16:21 ` Julien Grall
2017-08-22 17:06 ` Julien Grall
2017-08-13 21:30 ` [PATCH 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table mjaggi
` (3 subsequent siblings)
4 siblings, 2 replies; 12+ messages in thread
From: mjaggi @ 2017-08-13 21:30 UTC (permalink / raw)
To: xen-devel; +Cc: Andre.Przywara, julien.grall, sstabellini, Manish Jaggi
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_init.
Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
---
xen/arch/arm/gic-v3-its.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 2d36030..f844a0d 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -976,12 +976,31 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
return res;
}
+/* Common function for adding to host_its_list */
+static int add_to_host_its_list(u64 addr, u64 size, const void *node)
+{
+ struct host_its *its_data;
+ its_data = xzalloc(struct host_its);
+
+ if ( !its_data )
+ return -1;
+
+ its_data->addr = addr;
+ its_data->size = size;
+ if ( node )
+ its_data->dt_node = node;
+
+ printk("GICv3: Found ITS @0x%lx\n", addr);
+
+ list_add_tail(&its_data->entry, &host_its_list);
+
+ return 0;
+}
+
/* 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
* frames to the ITS list.
@@ -996,17 +1015,8 @@ 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);
+ if ( add_to_host_its_list(addr, size, its) )
+ panic("GICV3: Adding Host ITS failed ");
}
}
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list
2017-08-13 21:30 ` [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list mjaggi
@ 2017-08-22 16:21 ` Julien Grall
2017-08-22 17:06 ` Julien Grall
1 sibling, 0 replies; 12+ messages in thread
From: Julien Grall @ 2017-08-22 16:21 UTC (permalink / raw)
To: mjaggi, xen-devel; +Cc: Andre.Przywara, sstabellini, Manish Jaggi
Hello,
On 13/08/17 22:30, mjaggi@caviumnetworks.com wrote:
> 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_init.
>
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
> ---
> xen/arch/arm/gic-v3-its.c | 36 +++++++++++++++++++++++-------------
> 1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 2d36030..f844a0d 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -976,12 +976,31 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
> return res;
> }
>
> +/* Common function for adding to host_its_list */
> +static int add_to_host_its_list(u64 addr, u64 size, const void *node)
Why void *? node will be always assigned to dt_node and we should keep
some type safety.
Also this function only return -1 or 0. Please use boolean.
> +{
> + struct host_its *its_data;
Missing newline between the declaration and the code.
> + its_data = xzalloc(struct host_its);
> +
> + if ( !its_data )
> + return -1;
> +
> + its_data->addr = addr;
> + its_data->size = size;
> + if ( node )
This check is pointless. If it is NULL then dt_node will be NULL and
this is what we want.
> + its_data->dt_node = node;
> +
> + printk("GICv3: Found ITS @0x%lx\n", addr);
> +
> + list_add_tail(&its_data->entry, &host_its_list);
> +
> + return 0;
> +}
> +
> /* 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;
> -
Why this newline is dropped?
> /*
> * Check for ITS MSI subnodes. If any, add the ITS register
> * frames to the ITS list.
> @@ -996,17 +1015,8 @@ 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);
> + if ( add_to_host_its_list(addr, size, its) )
> + panic("GICV3: Adding Host ITS failed ");
> }
> }
>
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list
2017-08-13 21:30 ` [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list mjaggi
2017-08-22 16:21 ` Julien Grall
@ 2017-08-22 17:06 ` Julien Grall
1 sibling, 0 replies; 12+ messages in thread
From: Julien Grall @ 2017-08-22 17:06 UTC (permalink / raw)
To: mjaggi, xen-devel; +Cc: Andre.Przywara, sstabellini, Manish Jaggi
On 13/08/17 22:30, mjaggi@caviumnetworks.com wrote:
> 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_init.
>
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
> ---
> xen/arch/arm/gic-v3-its.c | 36 +++++++++++++++++++++++-------------
> 1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 2d36030..f844a0d 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -976,12 +976,31 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
> return res;
> }
>
> +/* Common function for adding to host_its_list */
> +static int add_to_host_its_list(u64 addr, u64 size, const void *node)
BTW this should be paddr_t and not u64 for both.
> +{
> + struct host_its *its_data;
> + its_data = xzalloc(struct host_its);
> +
> + if ( !its_data )
> + return -1;
> +
> + its_data->addr = addr;
> + its_data->size = size;
> + if ( node )
> + its_data->dt_node = node;
> +
> + printk("GICv3: Found ITS @0x%lx\n", addr);
> +
> + list_add_tail(&its_data->entry, &host_its_list);
> +
> + return 0;
> +}
> +
> /* 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
> * frames to the ITS list.
> @@ -996,17 +1015,8 @@ 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);
> + if ( add_to_host_its_list(addr, size, its) )
> + panic("GICV3: Adding Host ITS failed ");
> }
> }
>
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table
2017-08-13 21:30 [PATCH v2 0/5] [resend] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain mjaggi
2017-08-13 21:30 ` [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list mjaggi
@ 2017-08-13 21:30 ` mjaggi
2017-08-22 17:00 ` Julien Grall
2017-08-13 21:30 ` [PATCH 3/5] ARM: ITS: Deny hardware domain access to its mjaggi
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: mjaggi @ 2017-08-13 21:30 UTC (permalink / raw)
To: xen-devel; +Cc: Andre.Przywara, julien.grall, sstabellini, Manish Jaggi
From: Manish Jaggi <mjaggi@cavium.com>
Added gicv3_its_acpi_init to update host_its_list from MADT table.
For ACPI, host_its sturcture stores dt_node as NULL.
Future TOD0:
Cleanup :(1) Remove from host_its dt_node as it is required only for ACPI
Enhancement :(2) Provide a method to access translation_id and
other fields of madt generic translator.
Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
---
xen/arch/arm/gic-v3-its.c | 14 ++++++++++++++
xen/arch/arm/gic-v3.c | 8 ++++++++
xen/include/asm-arm/gic_v3_its.h | 13 +++++++++++++
3 files changed, 35 insertions(+)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index f844a0d..c4f1288 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -32,6 +32,7 @@
#include <asm/page.h>
#define ITS_CMD_QUEUE_SZ SZ_1M
+#define ACPI_GICV3_ITS_MEM_SIZE (SZ_64K)
/*
* No lock here, as this list gets only populated upon boot while scanning
@@ -1020,6 +1021,19 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
}
}
+#ifdef CONFIG_ACPI
+int gicv3_its_acpi_init(struct acpi_subtable_header *header,
+ const unsigned long end)
+{
+ struct acpi_madt_generic_translator *its;
+
+ its = (struct acpi_madt_generic_translator *)header;
+
+ return add_to_host_its_list(its->base_address,
+ ACPI_GICV3_ITS_MEM_SIZE, NULL);
+}
+#endif
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index f990eae..0be8942 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1567,6 +1567,14 @@ static void __init gicv3_acpi_init(void)
gicv3.rdist_stride = 0;
+ /* Parse ITS information */
+ count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+ gicv3_its_acpi_init, 0);
+
+ if ( count <= 0 )
+ panic("GICv3: Can't get ITS entry");
+
+
/*
* 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..2b7493d 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -105,6 +105,7 @@
#include <xen/device_tree.h>
#include <xen/rbtree.h>
+#include <xen/acpi.h>
#define HOST_ITS_FLUSH_CMD_QUEUE (1U << 0)
#define HOST_ITS_USES_PTA (1U << 1)
@@ -135,6 +136,10 @@ 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
+int gicv3_its_acpi_init(struct acpi_subtable_header *header,
+ const unsigned long end);
+#endif
bool gicv3_its_host_has_its(void);
unsigned int vgic_v3_its_count(const struct domain *d);
@@ -196,6 +201,14 @@ static inline void gicv3_its_dt_init(const struct dt_device_node *node)
{
}
+#ifdef CONFIG_ACPI
+static inline int gicv3_its_acpi_init(struct acpi_subtable_header *header,
+ const unsigned long end)
+{
+ return false;
+}
+#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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table
2017-08-13 21:30 ` [PATCH 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table mjaggi
@ 2017-08-22 17:00 ` Julien Grall
0 siblings, 0 replies; 12+ messages in thread
From: Julien Grall @ 2017-08-22 17:00 UTC (permalink / raw)
To: mjaggi, xen-devel; +Cc: Andre.Przywara, sstabellini, Manish Jaggi
Hello,
On 13/08/17 22:30, 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 sturcture stores dt_node as NULL.
s/sturcture/structure/
>
> Future TOD0:
> Cleanup :(1) Remove from host_its dt_node as it is required only for ACPI
> Enhancement :(2) Provide a method to access translation_id and
> other fields of madt generic translator.
I don't get those TODOs. This is not related to this patch and does not
really hence the commit message.
>
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
> ---
> xen/arch/arm/gic-v3-its.c | 14 ++++++++++++++
> xen/arch/arm/gic-v3.c | 8 ++++++++
> xen/include/asm-arm/gic_v3_its.h | 13 +++++++++++++
> 3 files changed, 35 insertions(+)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index f844a0d..c4f1288 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -32,6 +32,7 @@
> #include <asm/page.h>
>
> #define ITS_CMD_QUEUE_SZ SZ_1M
> +#define ACPI_GICV3_ITS_MEM_SIZE (SZ_64K)
The (...) are not necessary.
>
> /*
> * No lock here, as this list gets only populated upon boot while scanning
> @@ -1020,6 +1021,19 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
> }
> }
>
> +#ifdef CONFIG_ACPI
> +int gicv3_its_acpi_init(struct acpi_subtable_header *header,
> + const unsigned long end)
I don't much like the idea of providing a callback that will be called
by gic-v3.c. I would much prefer to follow the same pattern as the DT
where gic-v3.c will call a function in the gic-v3-its.c that will
iterate on all possible ITS.
This will make a more sane interface.
Also, it would make sense to call it gicv3_its_acpi_probe.
> +{
> + struct acpi_madt_generic_translator *its;
> +
> + its = (struct acpi_madt_generic_translator *)header;
You probably want to add check BAD_MADT_ENTRY(its, end).
> +
> + return add_to_host_its_list(its->base_address,
> + ACPI_GICV3_ITS_MEM_SIZE, NULL);
If you follow my suggestion in patch #1 regarding the return of
add_to_host_its_list, then you would need to check true/false and return
a correct errno.
Even if you don't follow it, please return an appropriate errno rather
than -1.
> +}
> +#endif
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index f990eae..0be8942 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1567,6 +1567,14 @@ static void __init gicv3_acpi_init(void)
>
> gicv3.rdist_stride = 0;
>
> + /* Parse ITS information */
> + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> + gicv3_its_acpi_init, 0);
See my comment above.
> +
> + if ( count <= 0 )
Hardware without ITS support will return 0 and therefore panic. You
don't want this to happen.
> + panic("GICv3: Can't get ITS entry");
> +
> +
> /*
> * 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..2b7493d 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -105,6 +105,7 @@
>
> #include <xen/device_tree.h>
> #include <xen/rbtree.h>
> +#include <xen/acpi.h>
With the suggestion suggested above, you will not need to include
<xen/acpi.h> in gic_v3_its.h.
>
> #define HOST_ITS_FLUSH_CMD_QUEUE (1U << 0)
> #define HOST_ITS_USES_PTA (1U << 1)
> @@ -135,6 +136,10 @@ 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
> +int gicv3_its_acpi_init(struct acpi_subtable_header *header,
> + const unsigned long end);
> +#endif
Newline here please.
> bool gicv3_its_host_has_its(void);
>
> unsigned int vgic_v3_its_count(const struct domain *d);
> @@ -196,6 +201,14 @@ static inline void gicv3_its_dt_init(const struct dt_device_node *node)
> {
> }
>
> +#ifdef CONFIG_ACPI
> +static inline int gicv3_its_acpi_init(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + return false;
gicv3_its_acpi_init return an int not a bool. Please modify this
accordingly.
> +}
> +#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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/5] ARM: ITS: Deny hardware domain access to its
2017-08-13 21:30 [PATCH v2 0/5] [resend] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain mjaggi
2017-08-13 21:30 ` [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list mjaggi
2017-08-13 21:30 ` [PATCH 2/5] ARM: ITS: Populate host_its_list from ACPI MADT Table mjaggi
@ 2017-08-13 21:30 ` mjaggi
2017-08-22 17:04 ` Julien Grall
2017-08-13 21:30 ` [PATCH 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations mjaggi
2017-08-13 21:30 ` [PATCH 5/5] ARM: ITS: Pass ITS in Hardware Domain MADT mjaggi
4 siblings, 1 reply; 12+ messages in thread
From: mjaggi @ 2017-08-13 21:30 UTC (permalink / raw)
To: xen-devel; +Cc: Andre.Przywara, julien.grall, sstabellini, Manish Jaggi
From: Manish Jaggi <mjaggi@cavium.com>
This patch extends the gicv3_iomem_deny_access functionality by adding support
for its region as well. Added function gicv3_its_deny_access.
Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
---
xen/arch/arm/gic-v3-its.c | 19 +++++++++++++++++++
xen/arch/arm/gic-v3.c | 7 +++++++
xen/include/asm-arm/gic_v3_its.h | 8 ++++++++
3 files changed, 34 insertions(+)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index c4f1288..f584d33 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -20,6 +20,7 @@
#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,24 @@ 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(ACPI_GICV3_ITS_MEM_SIZE);
+ rc = iomem_deny_access(d, mfn, mfn + nr);
+ if ( rc )
+ 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 0be8942..045d20d 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1308,6 +1308,13 @@ static int gicv3_iomem_deny_access(const struct domain *d)
if ( rc )
return rc;
+ if ( gicv3_its_host_has_its() )
+ {
+ rc = gicv3_its_deny_access(d);
+ if ( rc )
+ 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 b9d8957..a673fba 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -139,6 +139,9 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
int gicv3_its_acpi_init(struct acpi_subtable_header *header,
const unsigned long end);
#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);
@@ -208,6 +211,11 @@ static inline int gicv3_its_acpi_init(struct acpi_subtable_header *header,
}
#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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] ARM: ITS: Deny hardware domain access to its
2017-08-13 21:30 ` [PATCH 3/5] ARM: ITS: Deny hardware domain access to its mjaggi
@ 2017-08-22 17:04 ` Julien Grall
0 siblings, 0 replies; 12+ messages in thread
From: Julien Grall @ 2017-08-22 17:04 UTC (permalink / raw)
To: mjaggi, xen-devel; +Cc: Andre.Przywara, sstabellini, Manish Jaggi
Hello,
On 13/08/17 22:30, 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. Added function gicv3_its_deny_access.
s/its/ITS/ making clearer the commit message.
s/Added/Add/
>
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
> ---
> xen/arch/arm/gic-v3-its.c | 19 +++++++++++++++++++
> xen/arch/arm/gic-v3.c | 7 +++++++
> xen/include/asm-arm/gic_v3_its.h | 8 ++++++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index c4f1288..f584d33 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -20,6 +20,7 @@
>
> #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,24 @@ 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(ACPI_GICV3_ITS_MEM_SIZE);
> + rc = iomem_deny_access(d, mfn, mfn + nr);
> + if ( rc )
> + 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 0be8942..045d20d 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1308,6 +1308,13 @@ static int gicv3_iomem_deny_access(const struct domain *d)
> if ( rc )
> return rc;
>
> + if ( gicv3_its_host_has_its() )
gicv3_its_deny_access will do nothing and return 0 when there are no ITS
present. Same when Xen does not support ITS. So please drop this
pointless check.
> + {
> + rc = gicv3_its_deny_access(d);
> + if ( rc )
> + 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 b9d8957..a673fba 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -139,6 +139,9 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
> int gicv3_its_acpi_init(struct acpi_subtable_header *header,
> const unsigned long end);
> #endif
Newline here please.
> +/* 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);
> @@ -208,6 +211,11 @@ static inline int gicv3_its_acpi_init(struct acpi_subtable_header *header,
> }
> #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;
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations
2017-08-13 21:30 [PATCH v2 0/5] [resend] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain mjaggi
` (2 preceding siblings ...)
2017-08-13 21:30 ` [PATCH 3/5] ARM: ITS: Deny hardware domain access to its mjaggi
@ 2017-08-13 21:30 ` mjaggi
2017-08-22 17:17 ` Julien Grall
2017-08-13 21:30 ` [PATCH 5/5] ARM: ITS: Pass ITS in Hardware Domain MADT mjaggi
4 siblings, 1 reply; 12+ messages in thread
From: mjaggi @ 2017-08-13 21:30 UTC (permalink / raw)
To: xen-devel; +Cc: Andre.Przywara, julien.grall, sstabellini, Manish Jaggi
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 | 6 ++++++
xen/arch/arm/gic-v3-its.c | 12 ++++++++++++
xen/arch/arm/gic-v3.c | 17 +++++++++++++++++
xen/arch/arm/gic.c | 11 +++++++++++
xen/include/asm-arm/gic.h | 3 +++
xen/include/asm-arm/gic_v3_its.h | 6 ++++++
7 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1bec4fa..5739ea4 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1806,12 +1806,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..f5ca227 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1012,6 +1012,11 @@ static int gicv2_iomem_deny_access(const struct domain *d)
return iomem_deny_access(d, mfn, mfn + nr);
}
+static u32 gicv2_get_hwdom_madt_size(const struct domain *d)
+{
+ return 0;
+}
+
#ifdef CONFIG_ACPI
static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
{
@@ -1248,6 +1253,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-its.c b/xen/arch/arm/gic-v3-its.c
index f584d33..82e025e 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -924,6 +924,18 @@ int gicv3_its_deny_access(const struct domain *d)
return rc;
}
+#ifdef CONFIG_ACPI
+u32 gicv3_its_madt_generic_translator_size(void)
+{
+ const struct host_its *its_data;
+ u32 size = 0;
+
+ list_for_each_entry(its_data, &host_its_list, entry)
+ size += sizeof(struct acpi_madt_generic_translator);
+
+ return size;
+}
+#endif
/*
* 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 045d20d..6c2b562 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1410,6 +1410,17 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
return table_len;
}
+static u32 gicv3_get_hwdom_madt_size(const struct domain *d)
+{
+ u32 size;
+ size = sizeof(struct acpi_madt_generic_redistributor)
+ * d->arch.vgic.nr_regions;
+ if ( gicv3_its_host_has_its() )
+ size += gicv3_its_madt_generic_translator_size();
+
+ return size;
+}
+
static int __init
gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
const unsigned long end)
@@ -1607,6 +1618,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
{
return 0;
}
+
+static u32 gicv3_get_hwdom_madt_size(const struct domain *d)
+{
+ return 0;
+}
#endif
/* Set up the GIC */
@@ -1708,6 +1724,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..7bdb603 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -851,6 +851,17 @@ int gic_make_hwdom_madt(const struct domain *d, u32 offset)
return gic_hw_ops->make_hwdom_madt(d, offset);
}
+u32 gic_get_hwdom_madt_size(const struct domain *d)
+{
+ u32 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..a766e42 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 */
+ u32 (*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);
+u32 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);
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index a673fba..b849b16 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -138,6 +138,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
#ifdef CONFIG_ACPI
int gicv3_its_acpi_init(struct acpi_subtable_header *header,
const unsigned long end);
+u32 gicv3_its_madt_generic_translator_size(void);
#endif
/* Deny iomem access for its */
int gicv3_its_deny_access(const struct domain *d);
@@ -209,6 +210,11 @@ static inline int gicv3_its_acpi_init(struct acpi_subtable_header *header,
{
return false;
}
+
+static inline u32 gicv3_its_madt_generic_translator_size(void)
+{
+ 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations
2017-08-13 21:30 ` [PATCH 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations mjaggi
@ 2017-08-22 17:17 ` Julien Grall
0 siblings, 0 replies; 12+ messages in thread
From: Julien Grall @ 2017-08-22 17:17 UTC (permalink / raw)
To: mjaggi, xen-devel; +Cc: Andre.Przywara, sstabellini, Manish Jaggi
Hello,
On 13/08/17 22:30, 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 | 6 ++++++
> xen/arch/arm/gic-v3-its.c | 12 ++++++++++++
> xen/arch/arm/gic-v3.c | 17 +++++++++++++++++
> xen/arch/arm/gic.c | 11 +++++++++++
> xen/include/asm-arm/gic.h | 3 +++
> xen/include/asm-arm/gic_v3_its.h | 6 ++++++
> 7 files changed, 56 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 1bec4fa..5739ea4 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1806,12 +1806,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..f5ca227 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -1012,6 +1012,11 @@ static int gicv2_iomem_deny_access(const struct domain *d)
> return iomem_deny_access(d, mfn, mfn + nr);
> }
>
> +static u32 gicv2_get_hwdom_madt_size(const struct domain *d)
Please no more use of u32, use uint32_t. But, the return is a bit weird.
Why 32-bit when the code is using size_t.
I think this should return unsigned long given you use them in
combination with _xmalloc.
> +{
> + return 0;
> +}
> +
> #ifdef CONFIG_ACPI
> static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
> {
> @@ -1248,6 +1253,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-its.c b/xen/arch/arm/gic-v3-its.c
> index f584d33..82e025e 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -924,6 +924,18 @@ int gicv3_its_deny_access(const struct domain *d)
> return rc;
> }
>
> +#ifdef CONFIG_ACPI
> +u32 gicv3_its_madt_generic_translator_size(void)
See my comment above.
> +{
> + const struct host_its *its_data;
> + u32 size = 0;
Same here.
> +
> + list_for_each_entry(its_data, &host_its_list, entry)
> + size += sizeof(struct acpi_madt_generic_translator);
> +
> + return size;
> +}
Overall, I don't think this function is necessary. Instead you should
use vgic_v3_its_count given that the ACPI table will for the hardware
domain.
> +#endif
> /*
> * 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 045d20d..6c2b562 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1410,6 +1410,17 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
> return table_len;
> }
>
> +static u32 gicv3_get_hwdom_madt_size(const struct domain *d)
Ditto
> +{
> + u32 size;
Ditto + newline here.
> + size = sizeof(struct acpi_madt_generic_redistributor)
> + * d->arch.vgic.nr_regions;
> + if ( gicv3_its_host_has_its() )
> + size += gicv3_its_madt_generic_translator_size();
size += vgic_v3_its_count(d);
> +
> + return size;
> +}
> +
> static int __init
> gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
> const unsigned long end)
> @@ -1607,6 +1618,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
> {
> return 0;
> }
> +
> +static u32 gicv3_get_hwdom_madt_size(const struct domain *d)
> +{
> + return 0;
> +}
> #endif
>
> /* Set up the GIC */
> @@ -1708,6 +1724,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..7bdb603 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -851,6 +851,17 @@ int gic_make_hwdom_madt(const struct domain *d, u32 offset)
> return gic_hw_ops->make_hwdom_madt(d, offset);
> }
>
> +u32 gic_get_hwdom_madt_size(const struct domain *d)
See above for u32.
> +{
> + u32 madt_size;
Ditto + newline.
> + 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..a766e42 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 */
> + u32 (*get_hwdom_madt_size)(const struct domain *d);
See above for u32.
> /* 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);
> +u32 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);
>
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index a673fba..b849b16 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -138,6 +138,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
> #ifdef CONFIG_ACPI
> int gicv3_its_acpi_init(struct acpi_subtable_header *header,
> const unsigned long end);
> +u32 gicv3_its_madt_generic_translator_size(void);
Ditto.
> #endif
> /* Deny iomem access for its */
> int gicv3_its_deny_access(const struct domain *d);
> @@ -209,6 +210,11 @@ static inline int gicv3_its_acpi_init(struct acpi_subtable_header *header,
> {
> return false;
> }
> +
> +static inline u32 gicv3_its_madt_generic_translator_size(void)
> +{
> + return 0;
> +}
> #endif
>
> static inline int gicv3_its_deny_access(const struct domain *d)
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] ARM: ITS: Pass ITS in Hardware Domain MADT
2017-08-13 21:30 [PATCH v2 0/5] [resend] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain mjaggi
` (3 preceding siblings ...)
2017-08-13 21:30 ` [PATCH 4/5] ARM: Introduce get_hwdom_madt_size in gic_hw_operations mjaggi
@ 2017-08-13 21:30 ` mjaggi
2017-08-22 17:24 ` Julien Grall
4 siblings, 1 reply; 12+ messages in thread
From: mjaggi @ 2017-08-13 21:30 UTC (permalink / raw)
To: xen-devel; +Cc: Andre.Przywara, julien.grall, sstabellini, Manish Jaggi
From: Manish Jaggi <mjaggi@cavium.com>
Adds 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 | 24 ++++++++++++++++++++++++
xen/arch/arm/gic-v3.c | 1 +
xen/include/asm-arm/gic_v3_its.h | 1 +
3 files changed, 26 insertions(+)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 82e025e..6e0a701 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -935,6 +935,30 @@ u32 gicv3_its_madt_generic_translator_size(void)
return size;
}
+
+u32 gicv3_its_make_hwdom_madt(u8 *base_ptr, u32 offset)
+{
+ const struct host_its *its_data;
+ u32 table_len = offset, i = 0, size;
+ struct acpi_madt_generic_translator *fw_its;
+ struct acpi_madt_generic_translator *hwdom_its;
+
+ size = sizeof(struct acpi_madt_generic_translator);
+
+ /* Update GIC ITS information in hardware domain's MADT */
+ list_for_each_entry(its_data, &host_its_list, entry)
+ {
+ hwdom_its = (struct acpi_madt_generic_translator *)(base_ptr
+ + table_len);
+ fw_its = (struct acpi_madt_generic_translator *)
+ acpi_table_get_entry_madt(
+ ACPI_MADT_TYPE_GENERIC_TRANSLATOR, i++);
+ memcpy(hwdom_its, fw_its, size);
+ table_len += size;
+ }
+
+ return table_len;
+}
#endif
/*
* Create the respective guest DT nodes from a list of host ITSes.
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 6c2b562..30b29c9 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1407,6 +1407,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
table_len += size;
}
+ table_len = gicv3_its_make_hwdom_madt(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 b849b16..8955451 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -139,6 +139,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
int gicv3_its_acpi_init(struct acpi_subtable_header *header,
const unsigned long end);
u32 gicv3_its_madt_generic_translator_size(void);
+u32 gicv3_its_make_hwdom_madt(u8 *base_ptr, u32 offset);
#endif
/* Deny iomem access for its */
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 5/5] ARM: ITS: Pass ITS in Hardware Domain MADT
2017-08-13 21:30 ` [PATCH 5/5] ARM: ITS: Pass ITS in Hardware Domain MADT mjaggi
@ 2017-08-22 17:24 ` Julien Grall
0 siblings, 0 replies; 12+ messages in thread
From: Julien Grall @ 2017-08-22 17:24 UTC (permalink / raw)
To: mjaggi, xen-devel; +Cc: Andre.Przywara, sstabellini, Manish Jaggi
Hello,
Title: xen/arm: ITS: Expose ITS in the MADT table
On 13/08/17 22:30, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <mjaggi@cavium.com>
>
> Adds gicv3_its_make_hwdom_madt to update hwdom MADT ITS information.
s/Adds/Add/
>
> Signed-off-by: Manish Jaggi <mjaggi@cavium.com>
> ---
> xen/arch/arm/gic-v3-its.c | 24 ++++++++++++++++++++++++
> xen/arch/arm/gic-v3.c | 1 +
> xen/include/asm-arm/gic_v3_its.h | 1 +
> 3 files changed, 26 insertions(+)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 82e025e..6e0a701 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -935,6 +935,30 @@ u32 gicv3_its_madt_generic_translator_size(void)
>
> return size;
> }
> +
> +u32 gicv3_its_make_hwdom_madt(u8 *base_ptr, u32 offset)
Pretty much all my remarks in the previous patch for u* are valid here
too. I will not repeat them and for the whole the patch.
> +{
> + const struct host_its *its_data;
> + u32 table_len = offset, i = 0, size;
> + struct acpi_madt_generic_translator *fw_its;
> + struct acpi_madt_generic_translator *hwdom_its;
> +
> + size = sizeof(struct acpi_madt_generic_translator);
> +
> + /* Update GIC ITS information in hardware domain's MADT */
> + list_for_each_entry(its_data, &host_its_list, entry)
Please look at vgic_v3_its_count to avoid introduce dummy variables:
for ( i = 0; i < vgic_v3_its_count(...); i++ )
> + {
> + hwdom_its = (struct acpi_madt_generic_translator *)(base_ptr
> + + table_len);
> + fw_its = (struct acpi_madt_generic_translator *)
> + acpi_table_get_entry_madt(
> + ACPI_MADT_TYPE_GENERIC_TRANSLATOR, i++);
Please check the return here + panic if it does not work.
> + memcpy(hwdom_its, fw_its, size);
> + table_len += size;
This code is too complicate for not much reason. If you do:
hwdom_its = (struct acpi_madt_generic_translator)(base_ptr + offset);
for ( i = 0; i < vgic_v3_its_count(...); i++ )
{
fw_its = (struct .... *)acpi_table_get_...
hwdom_its++;
}
return (offset + sizeof(...) * vgic_v3_its_count());
That would be much clear.
> + }
> +
> + return table_len;
> +}
> #endif
> /*
> * Create the respective guest DT nodes from a list of host ITSes.
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 6c2b562..30b29c9 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1407,6 +1407,7 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
> table_len += size;
> }
>
> + table_len = gicv3_its_make_hwdom_madt(base_ptr, table_len);
Newline here.
> return table_len;
> }
>
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index b849b16..8955451 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -139,6 +139,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
> int gicv3_its_acpi_init(struct acpi_subtable_header *header,
> const unsigned long end);
> u32 gicv3_its_madt_generic_translator_size(void);
> +u32 gicv3_its_make_hwdom_madt(u8 *base_ptr, u32 offset);
> #endif
> /* Deny iomem access for its */
> int gicv3_its_deny_access(const struct domain *d);
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread