Re: [Xen-devel] [PATCH v3 3/5] ARM: ITS: Deny hardware domain access to ITS

[Manish Jaggi]

On 9/7/2017 10:27 PM, Andre Przywara wrote:

Hi,

On 05/09/17 18:14, mja...@caviumnetworks.com wrote:

From: Manish Jaggi 

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 
---
  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 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -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, _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;




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

Re: [Xen-devel] [PATCH v3 3/5] ARM: ITS: Deny hardware domain access to ITS

[Julien Grall]

On 07/09/17 17:57, Andre Przywara wrote:

Hi,


Hi,


On 05/09/17 18:14, mja...@caviumnetworks.com wrote:

From: Manish Jaggi 

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 
---
  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 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -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, _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,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 3/5] ARM: ITS: Deny hardware domain access to ITS

[Andre Przywara]

Hi,

On 05/09/17 18:14, mja...@caviumnetworks.com wrote:
> From: Manish Jaggi 
> 
> 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 
> ---
>  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 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -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, _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;
> 

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

[Xen-devel] [PATCH v3 3/5] ARM: ITS: Deny hardware domain access to ITS

[mjaggi]

From: Manish Jaggi 

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 
---
 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 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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, _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;
-- 
2.7.4


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