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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Feb 07, 2019 at 06:51:57PM +0100, Marek Marczykowski-Górecki wrote:
> 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.

Oh, that's good to know. So stubdomain will always be destroyed first,
making sure the irq permissions are removed from the stubdomain before
destroying the target domain and unmapping/destroying the irqs.

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

Not on domain destroy AFAICT.

What about hot-unplug? The proper flow there would be to ask the
stubdomain to detach the device, and only deassign it after the
stubdomain has reported successful detach. I think that's already the
case.

Thanks, Roger.

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