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

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 4/27/21 8:56 AM, Mahesh Salgaonkar 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.
> 
> [root@atest-guest ~]# dmesg | grep EEH
> [    0.032337] EEH: pSeries platform initialized
> [    0.298207] EEH: No capable adapters found: recovery disabled.
> [root@atest-guest ~]#
> 
> So far the linux kernel was assuming pe_config_addr equal to device's
> config_addr and using it to enable EEH on the PE through ibm,set-eeh-option
> RTAS call. Which wasn't the correct way as per PAPR. The linux kernel
> commit 98ba956f6a389 fixed this flow. With that fixed, linux now gets the
> PE config address first using the ibm,get-config-addr-info2 RTAS call, and
> then uses the found address as argument to ibm,set-eeh-option RTAS call to
> enable EEH on the PCI device. This works on PowerVM lpar but fails in qemu
> KVM guests. This is because ibm,set-eeh-option RTAS call in qemu expects PE
> config address bits 16:20 be populated with device number (DEVNUM).
> 
> The rtas call ibm,get-config-addr-info2 in qemu always returns the PE
> config address in form of "00BB0001" (i.e.  <00><BUS><DEVFN><REG>) where
> "BB" represents the bus number of PE's primary bus and with device number
> information always set to zero. However until commit 98ba956f6a389 this
> return value wasn't used to enable EEH on the PCI device.
> 
> Now in qemu guest with recent kernel the ibm,set-eeh-option RTAS call fails
> with -3 return value indicating that there is no PCI device exist for the
> specified pe config address. The rtas_ibm_set_eeh_option call uses
> pci_find_device() to get the PC device that matches specific bus and devfn
> extracted from PE config address passed as argument. Since the DEVFN part
> of PE config always contains zero, pci_find_device() fails to find the
> specific PCI device and hence fails to enable the EEH capability.
> 
> hw/ppc/spapr_pci_vfio.c: spapr_phb_vfio_eeh_set_option()
>     case RTAS_EEH_ENABLE: {
>          PCIHostState *phb;
>          PCIDevice *pdev;
> 
>          /*
>           * The EEH functionality is enabled on basis of PCI device,
>           * instead of PE. We need check the validity of the PCI
>           * device address.
>           */
>          phb = PCI_HOST_BRIDGE(sphb);
>          pdev = pci_find_device(phb->bus,
>                                 (addr >> 16) & 0xFF, (addr >> 8) & 0xFF);
>          if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
>              return RTAS_OUT_PARAM_ERROR;
>          }
> 
> hw/pci/pci.c:pci_find_device()
> 
> PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
> {
>      bus = pci_find_bus_nr(bus, bus_num);
> 
>      if (!bus)
>          return NULL;
> 
>      return bus->devices[devfn];
> }
> 
> This patch fixes this issue by populating DEVNUM field (bits 16:20) of PE
> config address with device number.
> 
> After this fix guest is able to find EEH capable devices and enable EEH
> recovery on it.
> 
> [root@atest-guest ~]# dmesg | grep EEH
> [    0.048139] EEH: pSeries platform initialized
> [    0.405115] EEH: Capable adapter found: recovery enabled.
> [root@atest-guest ~]#
> 
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/ppc/spapr_pci.c |    8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index feba18cb12..d6b0c380c8 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -538,8 +538,9 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
>       }
>   
>       /*
> -     * We always have PE address of form "00BB0001". "BB"
> -     * represents the bus number of PE's primary bus.
> +     * Return PE address of form "00BBD001". "BB" represents the
> +     * bus number of PE's primary bus and "D" represents the device number.
> +     * Guest uses this PE address to enable EEH on this pci device.
>        */
>       option = rtas_ld(args, 3);
>       switch (option) {
> @@ -550,7 +551,8 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
>               goto param_error_exit;
>           }
>   
> -        rtas_st(rets, 1, (pci_bus_num(pci_get_bus(pdev)) << 16) + 1);
> +        rtas_st(rets, 1, (pci_bus_num(pci_get_bus(pdev)) << 16) |
> +                         (PCI_DEVFN(PCI_SLOT(pdev->devfn), 0) << 8) | 1);
>           break;
>       case RTAS_GET_PE_MODE:
>           rtas_st(rets, 1, RTAS_PE_MODE_SHARED);
> 
>