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

[Manish Jaggi <mjaggi@xxxxxxxxxxxxxxxxxx>]

On 9/7/2017 10:27 PM, Andre Przywara wrote:
> 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.
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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel