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

Jan Kiszka
2012-01-17T13:08:37Z

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