| Message ID | 1507616218-2478-4-git-send-email-mjaggi@caviumnetworks.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Manish, I just spotted an issue in this patch. On 10/10/17 07:16, 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. > > Reviewed-by: Andre Przywara <andre.przywara@arm.com> > Acked-by: Julien Grall <julien.grall@arm.com> > 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 3023ee5..7746ae8 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); While you fix the bug (see below), please use its_data->size here. > + rc = iomem_deny_access(d, mfn, mfn + nr); > + if ( rc ) > + { > + printk( "iomem_deny_access failed for %lx:%lx \r\n", mfn, nr); And here the spurious space between ( and ". > + 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; you return rc here in case of an error, however you don't set it before. So this will always return 0 and Xen will continue to boot as nothing happen. The best way to fix it would be: 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 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; > Cheers,
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 3023ee5..7746ae8 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;