Re: [Xen-devel] [PATCH v4 4/6] xen/x86: Allow stubdom access to irq created for msi.

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Feb 07, 2019 at 06:40:16PM +0100, Roger Pau Monné wrote:
> On Thu, Feb 07, 2019 at 04:41:38PM +0100, Marek Marczykowski-Górecki wrote:
> > On Thu, Feb 07, 2019 at 03:57:54PM +0100, Roger Pau Monné wrote:
> > > On Thu, Feb 07, 2019 at 03:52:38PM +0100, Marek Marczykowski-Górecki 
> > > wrote:
> > > > Hmm, looking at the code, wouldn't it make sense to give device model
> > > > domain access to the IRQ _instead of_ hardware domain? If stubdomain is
> > > > in use, I don't see why dom0 would need access to that irq. Simply
> > > > provide what the device model domain is as parameter - either
> > > > hardware_domain, or stubdomain. Something like:
> > > > 
> > > >     create_irq(..., current->domain->target == d ? current->domain : 
> > > > hardware_domain);
> > > 
> > > Isn't there some cleanup that likely needs to be done by dom0 if it's
> > > not done by the stubdom, or in case the stubdom crashes for some
> > > reason?
> > 
> > I don't think toolstack know anything about IRQs allocated by device
> > model, looks like it does cleanup only for INTx interrupts.
> > 
> > > Or maybe that's already done on domain destruction by Xen itself, in
> > > which case not giving permissions to dom0 would be fine.
> > 
> > There is free_domain_pirqs() call in arch_domain_destroy(). But I don't
> > have device model reference there. Is there a way to get target ->
> > stubdomain mapping (other than iterating over all the domains)? I see
> > also domain->target field, which is the other way around.
> > The only thing needed is irq_deny_access() call there (in case of domain
> > ID reuse). Since such IRQs are not mapped to stubdomain itself,
> > free_domain_pirqs() for stubdomain will not clean this up.
> > Or maybe, _if stubdomain is guaranteed to be destroyed before its
> > target_, we can iterate over target domain's IRQs during stubdomain
> > destruction for this purpose?
> 
> The list of allowed irqs is stored inside of the domain struct,
> which means that it goes away when the domain is destroyed, there's no
> need to do any specific cleanup when the stubdomain is destroyed
> AFAICT. Now if the target domain is destroyed, those permissions over
> the irqs must be removed from the stubdomain, because the irqs will be
> freed and likely reused. The current model assumes that the hardware
> domain is always the controlling owner of such irqs, but if we allow
> stubdomains to also be the controlling owner then we need to keep some
> track of this, or else Xen could be leaking permissions.

This looks to be not a problem, because stubdomain keeps reference to
its target domain (and there is corresponding put_domain(d->target) on
domain destroy), so to answer my own question "if stubdomain is
guaranteed to be destroyed before its target" - yes, it is.

So, if domain destruction also implicitly revoke all _its_ irq
permissions (because of where they are stored), there is no additional
cleanup needed here.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Attachment: signature.asc
Description: PGP signature

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