Re: [PATCH] pci-assign: Fix multifunction support

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-01-16 18:11, Alex Williamson wrote:
> The core PCI code sets the multifunction bit in the header before
> calling the device initfn.  For device assignment, we're blasting
> that value with the actual hardware value, so nobody sees the
> additional functions if the devices isn't physically multifunction.
> Switch the HEADER_TYPE to a fully emulated field (all read-only
> anyway) and add setting and clearing of the multifunction bit to
> match qemu directive.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
>  hw/device-assignment.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 2a9e66d..7f4a5ec 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -540,6 +540,13 @@ again:
>          fprintf(stderr, "%s: read failed, errno = %d\n", __func__, errno);
>      }
>  
> +    /* Restore or clear multifunction, this is always controlled by qemu */
> +    if (pci_dev->dev.cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> +        pci_dev->dev.config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
> +    } else {
> +        pci_dev->dev.config[PCI_HEADER_TYPE] &= ~PCI_HEADER_TYPE_MULTI_FUNCTION;
> +    }
> +

Why have this in get_*real*_device? Why not fix this up at the caller
site, i.e. in assigned_initfn? Just for consistency, not a functional issue.

>      /* Clear host resource mapping info.  If we choose not to register a
>       * BAR, such as might be the case with the option ROM, we can get
>       * confusing, unwritable, residual addresses from the host here. */
> @@ -1575,7 +1582,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
>      assigned_dev_direct_config_read(dev, PCI_CLASS_PROG, 3);
>      assigned_dev_direct_config_read(dev, PCI_CACHE_LINE_SIZE, 1);
>      assigned_dev_direct_config_read(dev, PCI_LATENCY_TIMER, 1);
> -    assigned_dev_direct_config_read(dev, PCI_HEADER_TYPE, 1);
>      assigned_dev_direct_config_read(dev, PCI_BIST, 1);
>      assigned_dev_direct_config_read(dev, PCI_CARDBUS_CIS, 4);
>      assigned_dev_direct_config_read(dev, PCI_SUBSYSTEM_VENDOR_ID, 2);
> 

Looks good otherwise. Is it a regression of the access control refactoring?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux