| Message ID | 1504631700-19358-4-git-send-email-mjaggi@caviumnetworks.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 05/09/17 18:14, 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> > --- > 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 536b48d..0ab1466 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> > @@ -906,6 +907,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(ACPI_GICV3_ITS_MEM_SIZE); Shouldn't this not only cover the ITS register frame, but also the following 64K page containing the doorbell address? Otherwise we leave the doorbell address open, which seems to be asking for trouble ... Cheers, Andre. > + 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 993819a..9cf18da 100644 > --- a/xen/include/asm-arm/gic_v3_its.h > +++ b/xen/include/asm-arm/gic_v3_its.h > @@ -138,6 +138,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); > @@ -205,6 +209,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; >
On 07/09/17 17:57, Andre Przywara wrote: > Hi, Hi, > On 05/09/17 18:14, 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> >> --- >> 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 536b48d..0ab1466 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> >> @@ -906,6 +907,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(ACPI_GICV3_ITS_MEM_SIZE); > > Shouldn't this not only cover the ITS register frame, but also the > following 64K page containing the doorbell address? Otherwise we leave > the doorbell address open, which seems to be asking for trouble ... I think you are right. We don't want to allow the hardware domain to map the doorbell itself. This should only be done by Xen. Cheers,
On 9/7/2017 10:27 PM, Andre Przywara wrote: > Hi, > > On 05/09/17 18:14, 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> >> --- >> 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 536b48d..0ab1466 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> >> @@ -906,6 +907,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(ACPI_GICV3_ITS_MEM_SIZE); > Shouldn't this not only cover the ITS register frame, but also the > following 64K page containing the doorbell address? Otherwise we leave > the doorbell address open, which seems to be asking for trouble ... > > Cheers, > Andre. ok, I will fix in patch 2 the size as 128K, same a linux. If no other change required in this patch can you please ack it. > >> + 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 993819a..9cf18da 100644 >> --- a/xen/include/asm-arm/gic_v3_its.h >> +++ b/xen/include/asm-arm/gic_v3_its.h >> @@ -138,6 +138,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); >> @@ -205,6 +209,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; >>
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 536b48d..0ab1466 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> @@ -906,6 +907,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(ACPI_GICV3_ITS_MEM_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 993819a..9cf18da 100644 --- a/xen/include/asm-arm/gic_v3_its.h +++ b/xen/include/asm-arm/gic_v3_its.h @@ -138,6 +138,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); @@ -205,6 +209,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;