Re: [PATCH] spapr: Modify ibm, get-config-addr-info2 to set DEVNUM in PE config address.

[Mahesh J Salgaonkar]

On 2021-04-30 11:53:24 Fri, Oliver O'Halloran wrote:
> On Thu, Apr 29, 2021 at 7:02 PM Mahesh J Salgaonkar
> <mahesh@linux.ibm.com> wrote:
> >
> > On 2021-04-28 22:33:45 Wed, Oliver O'Halloran wrote:
> > > On Tue, Apr 27, 2021 at 9:56 PM Mahesh Salgaonkar <mahesh@linux.ibm.com> 
> > > wrote:
> > > >
> > > > With upstream kernel, especially after commit 98ba956f6a389
> > > > ("powerpc/pseries/eeh: Rework device EEH PE determination") we see that 
> > > > KVM
> > > > guest isn't able to enable EEH option for PCI pass-through devices 
> > > > anymore.
> > >
> > > How are you passing the devices through to the guest?
> >
> > I am using libvirt with below xml section to add pass-through:
> >
> >     <hostdev mode='subsystem' type='pci' managed='yes'>
> >       <driver name='vfio'/>
> >       <source>
> >         <address domain='0x0033' bus='0x01' slot='0x00' function='0x0'/>
> >       </source>
> >       <address type='pci' domain='0x0000' bus='0x01' slot='0x01' 
> > function='0x0' multifunction='on'/>
> >     </hostdev>
> >     <hostdev mode='subsystem' type='pci' managed='yes'>
> >       <driver name='vfio'/>
> >       <source>
> >         <address domain='0x0033' bus='0x01' slot='0x00' function='0x1'/>
> >       </source>
> >       <address type='pci' domain='0x0000' bus='0x01' slot='0x01' 
> > function='0x1' multifunction='on'/>
> >     </hostdev>
> >
> > Looks like libvirt does not allow pass through device in slot zero, and
> > throws following error.
> >
> > error: XML error: Invalid PCI address 0000:01:00.0. slot must be >= 1
> > Failed. Try again? [y,n,i,f,?]:
> 
> That's pretty odd and I have no idea why that's happening. I seem to
> remember being able to use slot 0 for vfio devices when doing the
> passthru manually with the qemu command line so this might be a
> libvirt quirk.
> 
> > *snip*
> >
> > Agree. I realize my fix is not correctly handling this. The current code
> > under ibm,set-eeh-option is checking for individual PCI device presence.
> > Better fix should be to check if there is any PCI device (vfio-pci)
> > present under specified bus and enable the EEH if found. And no change
> > in return value of get-config-addr-info2. What do you say ?
> 
> That sounds reasonable. You would however need to verify that all the
> devices on that bus are within the same PE on the hypervisor side.

I see that the spapr_phb_eeh_available(sphb) check in ibm,set-eeh-option
already makes sure that all the devices on that bus are from same iommu
group (within same PE) before going ahead with enabling EEH.

---------
rtas_ibm_set_eeh_option():
    ...
    if (!spapr_phb_eeh_available(sphb)) {          <---
        goto param_error_exit;
    }

    ret = spapr_phb_vfio_eeh_set_option(sphb, addr, option);
    rtas_st(rets, 0, ret);
    return;

param_error_exit:
    rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
----------

I verified that, if we add devices from two different PHB under same
sphb then spapr_phb_eeh_available(sphb) returns false and we fail to
enable EEH. Hence we are good here, we fail very early if devices on
that bus are not within same PE.

So, I just need to check for presence of any vfio-pci device under the
specified bus and enable the EEH. Let me know your views on this.

Thanks,
-Mahesh.

-- 
Mahesh J Salgaonkar