Re: [Xen-devel] xsa46-4.2.patch breaks PCI passthrough?

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.05.13 at 12:43, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Thu, 2013-05-02 at 09:49 +0100, Jan Beulich wrote:
>> >>> On 01.05.13 at 13:28, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>> >> XSA-46 was to do with PCI passthrough of PV domains, and in particular
>> >> changing some of the rules regarding interrupts.
>> 
>> This was misguiding me - I somehow concluded that the problems
>> here are being observed with PV domains, but considering the
>> second report we got as well as looking through the log files I'm
>> now rather guessing that the problem is (only) with HVM domains.
>> That in turn would match up with the code in pciif.py:
>> 
>>         if not self.vm.info.is_hvm() and dev.irq:
>>             rc = xc.physdev_map_pirq(domid = fe_domid,
>>                                    index = dev.irq,
>>                                    pirq  = dev.irq)
>>             if rc < 0:
>>                 raise VmError(('pci: failed to map irq on device '+
>>                             '%s - errno=%d')%(dev.name,rc))
>>         if dev.irq>0:
>>             log.debug('pci: enabling irq %d'%dev.irq)
>>             rc = xc.domain_irq_permission(domid =  fe_domid, pirq = dev.irq,
>>                     allow_access = True)
>>             if rc<0:
>>                 raise VmError(('pci: failed to configure irq on device '+
>>                             '%s - errno=%d')%(dev.name,rc))
>> 
>> i.e. the first portion of the setup is only being done for PV
>> guests. I have no idea why this is so (irqif.py doesn't special
>> case the guest kind, nor does libxl). Quite likely dropping that
>> check would be sufficient, but of course that should be
>> confirmed by someone knowing that code (and ideally also
>> knowing why this was being special cased in the first place) -
>> Ian, Ian?
> 
> If you are asking me why xend behaves this way then I have no clue.
> Finding someone who does is probably a big ask, unless the changelog
> offers any clues, the commit in question seems to be:
> 
>         commit 345fbe6cb410fb43c7b269a54d1c60e1e025f393
>         Author: Keir Fraser <keir.fraser@xxxxxxxxxx>
>         Date:   Mon Sep 7 08:38:39 2009 +0100
>         
>             xend: passthrough: fix physdev_map_pirq invocation
>             
>             For those devices not having INTx (like VFs), avoid calling 
> map_pirq,
>             otherwise the guest cannot be started successfully.
>             
>             Also avoid calling this hypercall for hvm guest, this is done in 
> the
>             device model.
>             
>             Signed-off-by: Qing He <qing.he@xxxxxxxxx>
> 
> Seems like "For those devices" is the "and dev.irq" bit and the "Also
> avoid" is the "is_hvm()" bit. I have no idea about the validity of any
> of that reasoning though...

I think I agree with this interpretation, and on that basis I just
went through the involved hypervisor side code path - afaict
there should be no problem with this being done in xend and
then a second time in the device model. Therefore I think we
ought to see whether the suggested adjustment actually works
for the reporters of the problem, and just go with it if so.

Jan


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