Re: [Xen-devel] [RFC PATCH 30/49] ARM: new VGIC: Add ENABLE registers handlers

[Julien Grall <julien.grall@xxxxxxx>]

On 19/02/18 12:41, Andre Przywara wrote:
> Hi,
Hi,

> On 16/02/18 16:57, Julien Grall wrote:
> > On 09/02/18 14:39, Andre Przywara wrote:
> > > +    spin_lock_irqsave(&desc->lock, flags);
> > > +    if ( enable )
> > > +    {
> > > +        gic_set_irq_type(desc, irq_type == VGIC_CONFIG_LEVEL ?
> > > +                 IRQ_TYPE_LEVEL_HIGH : IRQ_TYPE_EDGE_RISING);
> > Indentation and I would prefer a helper to convert between the vgic
> > value and the IRQ_TYPE. This would make the code easier to read.
> >
> > Also, this code does not replicate correctly the current vGIC.
> > gic_set_irq_type is only allowed to be used when
> > irq_set_type_by_domain(d) returns true. If you consider this change
> > valid, then I would like to know why.
> So what is/was the rationale for not allowing IRQ type changes for
> non-privileged guests? If you allow to pass through an hardware IRQ to a
> guest (which is the case this function handles), then I don't see why a
> guest would not be allowed to change the configuration? It seems rather
> odd, I guess it's up to the guest to know which type of IRQ this is?
If you can answer the question on top of irq_type_set_by_domain (i.e 
"See whether it is possible to let any domain configure the type) then 
we can remove it. We decided to only allow for the hardware domain 
because we trust it.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel