Re: [PATCH v6 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions

[Shannon Zhao <shannon.zhao@linaro.org>]

On 2016年03月24日 20:45, Stefano Stabellini wrote:
> On Tue, 22 Mar 2016, Julien Grall wrote:
>> > Hi Shannon,
>> > 
>> > On 17/03/16 09:41, Shannon Zhao wrote:
>>> > > From: Shannon Zhao <shannon.zhao@linaro.org>
>>> > > 
>>> > > Add a new member in gic_hw_operations which is used to deny Dom0 access
>>> > > to GIC regions.
>>> > > 
>>> > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> > > ---
>>> > > v6: use SZ_64K for GICv3 distributor
>>> > > ---
>>> > >   xen/arch/arm/gic-v2.c     | 31 +++++++++++++++++++++++++++++++
>>> > >   xen/arch/arm/gic-v3.c     | 45
>>> > > +++++++++++++++++++++++++++++++++++++++++++++
>>> > >   xen/arch/arm/gic.c        |  5 +++++
>>> > >   xen/include/asm-arm/gic.h |  3 +++
>>> > >   4 files changed, 84 insertions(+)
>>> > > 
>>> > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
>>> > > index 02db5f2..186f944 100644
>>> > > --- a/xen/arch/arm/gic-v2.c
>>> > > +++ b/xen/arch/arm/gic-v2.c
>>> > > @@ -22,6 +22,7 @@
>>> > >   #include <xen/init.h>
>>> > >   #include <xen/mm.h>
>>> > >   #include <xen/irq.h>
>>> > > +#include <xen/iocap.h>
>>> > >   #include <xen/sched.h>
>>> > >   #include <xen/errno.h>
>>> > >   #include <xen/softirq.h>
>>> > > @@ -714,6 +715,31 @@ static u32 gicv2_make_hwdom_madt(const struct domain
>>> > > *d, u32 offset)
>>> > >       return table_len;
>>> > >   }
>>> > > 
>>> > > +static int gicv2_iomem_deny_access(const struct domain *d)
>>> > > +{
>>> > > +    int rc;
>>> > > +    unsigned long gfn, nr;
>>> > > +
>>> > > +    gfn = dbase >> PAGE_SHIFT;
>>> > > +    rc = iomem_deny_access(d, gfn, gfn + 1);
>>> > > +    if ( rc )
>>> > > +        return rc;
>>> > > +
>>> > > +    gfn = hbase >> PAGE_SHIFT;
>>> > > +    rc = iomem_deny_access(d, gfn, gfn + 1);
>>> > > +    if ( rc )
>>> > > +        return rc;
>>> > > +
>>> > > +    gfn = cbase >> PAGE_SHIFT;
>>> > > +    nr = DIV_ROUND_UP(csize, PAGE_SIZE);
>>> > > +    rc = iomem_deny_access(d, gfn, gfn + nr);
>>> > > +    if ( rc )
>>> > > +        return rc;
>>> > > +
>>> > > +    gfn = vbase >> PAGE_SHIFT;
>>> > > +    return iomem_deny_access(d, gfn, gfn + nr);
>>> > > +}
>>> > > +
>>> > >   static int __init
>>> > >   gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
>>> > >                           const unsigned long end)
>>> > > @@ -809,6 +835,10 @@ static u32 gicv2_make_hwdom_madt(const struct domain
>>> > > *d, u32 offset)
>>> > >   {
>>> > >       return 0;
>>> > >   }
>>> > > +static int gicv2_iomem_deny_access(const struct domain *d)
>>> > > +{
>>> > > +    return 0;
>>> > > +}
>> > 
>> > I don't see any benefits to have iomem_deny_access only implemented when
>> > CONFIG_ACPI is built.
>> > 
>> > Because in this case, you will also deny the iomem when Xen is booting using
>> > device tree.
> That's true, it would be better to do that for device tree too.
> 
Ok, I'll move it out of the CONFIG_ACPI. But calling it for device tree
would be another patch I think.

Thanks,
-- 
Shannon

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