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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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:
> > On 01/05/13 12:09, Andrew Cooper wrote:
> >> On 01/05/13 06:29, Steven Haigh wrote:
> >>> Hi all,
> >>>
> >>> I've had a report lodged against my packages that the patch provided for 
> >>> XSA46 against Xen 4.2.1 causes PCI passthru to break.
> >>>
> >>> It seems that 4.2.1 *without* the XSA46 patch works perfectly. 4.2.2 
> >>> does not work.
> >>>
> >>> I added this patch in xen-4.2.1-6 of my RPMs (http://xen.crc.id.au) and 
> >>> the reporter has built the same SRPM with xsa46 patch removed and PCI 
> >>> passthrough works as intended.
> >>>
> >>> Reapplying the XSA46 patch causes it to break again.
> >>>
> >>> The bug report and logs can be found here:
> >>>   http://xen.crc.id.au/bugs/view.php?id=5 
> >>>
> >>> Has anyone come across this?
> >>>
> >> 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...

Ian.


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