| Message ID | 1502659815-20397-3-git-send-email-mjaggi@caviumnetworks.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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,
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;