Re: [Xen-devel] [PATCH v6 1/3] xen/pt: fix some pass-thru devices don't work across reboot

[Chao Gao <chao.gao@xxxxxxxxx>]

On Fri, Jan 25, 2019 at 09:13:49AM -0700, Jan Beulich wrote:
>>>> On 25.01.19 at 09:26, <chao.gao@xxxxxxxxx> wrote:
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -732,7 +732,11 @@ long arch_do_domctl(
>>              break;
>>  
>>          ret = -EPERM;
>> -        if ( irq <= 0 || !irq_access_permitted(currd, irq) )
>> +        /*
>> +         * irq < 0 denotes the corresponding pirq has been forcibly unbound.
>> +         * For this case, bypass permission check to reap the pirq.
>> +         */
>> +        if ( !irq || ((irq > 0) && !irq_access_permitted(currd, irq)) )
>>              break;
>
>So why would it be correct to continue into pt_irq_destroy_bind()
>with irq < 0?  And with an actual XSM policy I'm not sure you'd
>even make it past xsm_unbind_pt_irq(). If the IRQ was forcibly
>unbound before, there shouldn't be anything left to clean up?

But some hints are left to denote that a pirq was forcibly unbound.
See the code snippets below:

''' in unmap_domain_pirq()
        ...
        if ( !forced_unbind )
            clear_domain_irq_pirq(d, irq, info);
        else
        {
            info->arch.irq = -irq;
            radix_tree_replace_slot(
                radix_tree_lookup_slot(&d->arch.irq_pirq, irq),
                radix_tree_int_to_ptr(-pirq));
        }
        ...
'''

and 

''' in pirq_guest_unbind()
    ...
    if ( desc == NULL )
    {
        irq = -pirq->arch.irq;
        BUG_ON(irq <= 0);
        desc = irq_to_desc(irq);
        spin_lock_irq(&desc->lock);
        clear_domain_irq_pirq(d, irq, pirq);
    }
    ...
'''

>
>On the whole I think all the extra additions in v6 only serve to
>mask the tool stack not needing to do anymore some of what it
>does, as suggested in a reply to an earlier version. So I guess I
>agree with Roger that v5 came closer, but may need to be
>amended by some tool stack adjustment(s).

Yes. What v6 tries to mask here is irq unbinding and irq unmapping invoked
by qemu and pciback. We need to fix them in pciback and qemu rather than
tool stack. If we want to leave those error messages alone, we can just
take Roger's suggestion. Otherwise, we should try to conditional bypass
irq unbinding and irq unmapping in pciback and qemu and justify it.

Thanks
Chao

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