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

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 05/09/17 18:14, mjaggi@xxxxxxxxxxxxxxxxxx wrote:
> From: Manish Jaggi <mjaggi@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
>  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;
> 

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