RE: [PATCH] vfio/pci: Add pba_offset PCI quirk for BAIDU KUNLUN AI processor

["Cai,Huoqing" <caihuoqing@baidu.com>]

Sure I will check these issue in our hardware
Then ,send patch-v2 

-----Original Message-----
From: Alex Williamson <alex.williamson@redhat.com> 
Sent: 2021年7月13日 3:49
To: Cai,Huoqing <caihuoqing@baidu.com>
Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; qemu-devel@nongnu.org
Subject: Re: [PATCH] vfio/pci: Add pba_offset PCI quirk for BAIDU KUNLUN AI processor

On Mon, 12 Jul 2021 11:36:55 +0800
Cai Huoqing <caihuoqing@baidu.com> wrote:

> Fix pba_offset initialization value for BAIDU KUNLUN Virtual Function 
> device. The KUNLUN hardware returns an incorrect value for the VF PBA 
> offset, and add a quirk to instead return a hardcoded value of 0xb400.
> 
> Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
> ---
>  hw/vfio/pci.c            | 8 ++++++++
>  include/hw/pci/pci_ids.h | 4 ++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 
> ab4077aad2..72b7abf623 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1499,6 +1499,14 @@ static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
>          if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
>              (vdev->device_id & 0xff00) == 0x5800) {
>              msix->pba_offset = 0x1000;
> +        /*
> +         * BAIDU KUNLUN Virtual Function devices are encoded as 0x3685 for
> +         * KUNLUN AI processor. The KUNLUN hardware returns an incorrect
> +         * value for the VF PBA offset. The correct value is 0xb400.
> +         */

What is the incorrect value and what is the BAR size?  This information in the comment could help debugging later.

> +        } else if (vdev->vendor_id == PCI_VENDOR_ID_BAIDU &&
> +                   vdev->device_id == PCI_DEVICE_ID_KUNLUN_VF) {

Since we don't have an "encoding" like the previous quirk, we can use
vfio_pci_is() here:

        } else if (vfio_pci_is(vdev, PCI_VENDOR_ID_BAIDU,
                               PCI_DEVICE_ID_KUNLUN_VF)) {

> +            msix->pba_offset = 0xb400;
>          } else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
>              error_setg(errp, "hardware reports invalid configuration, "
>                         "MSIX PBA outside of specified BAR"); diff 
> --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h index 
> 5c14681b82..bc73c50277 100644
> --- a/include/hw/pci/pci_ids.h
> +++ b/include/hw/pci/pci_ids.h
> @@ -227,6 +227,10 @@
>  #define PCI_VENDOR_ID_FREESCALE          0x1957
>  #define PCI_DEVICE_ID_MPC8533E           0x0030
>  
> +#define PCI_VENDOR_ID_BAIDU              0x1d22
> +#define PCI_DEVICE_ID_KUNLUN             0x3684

Let's not add a device ID that we don't use elsewhere in the code, we only use the vendor ID and the VF ID below.  Thanks,

Alex

> +#define PCI_DEVICE_ID_KUNLUN_VF          0x3685
> +
>  #define PCI_VENDOR_ID_INTEL              0x8086
>  #define PCI_DEVICE_ID_INTEL_82378        0x0484
>  #define PCI_DEVICE_ID_INTEL_82441        0x1237