On Thu, Jun 17, 2010 at 03:15:51PM +0900, Isaku Yamahata wrote:
> set PCI multi-function bit appropriately.
>
> Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp>
>
> ---
> changes v1 -> v2:
> don't set header type register in configuration space.
> ---
> hw/pci.c | 25 +++++++++++++++++++++++++
> 1 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 5316aa5..ee391dc 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -607,6 +607,30 @@ static void pci_init_wmask_bridge(PCIDevice *d)
> pci_set_word(d->wmask + PCI_BRIDGE_CONTROL, 0xffff);
> }
>
> +static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> +{
> + uint8_t slot = PCI_SLOT(dev->devfn);
> + uint8_t func_max = 8;
enum or define would be better
> + uint8_t func;
If I understand correctly what this does, it goes over
other functions of the same device, and sets the MULTI_FUNCTION bit
for them if there's more than one function.
Instead, why don't we just set PCI_HEADER_TYPE_MULTI_FUNCTION
in relevant devices?
> +
> + for (func = 0; func < func_max; ++func) {
> + if (bus->devices[PCI_DEVFN(slot, func)]) {
> + break;
> + }
> + }
> + if (func == func_max) {
> + return;
> + }
> +
The above only works if the function is called before
device is added to bus.
> + for (func = 0; func < func_max; ++func) {
> + if (bus->devices[PCI_DEVFN(slot, func)]) {
> + bus->devices[PCI_DEVFN(slot, func)]->config[PCI_HEADER_TYPE] |=
> + PCI_HEADER_TYPE_MULTI_FUNCTION;
> + }
> + }
> + dev->config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
Isn't the bit set above already?
> +}
> +
> static void pci_config_alloc(PCIDevice *pci_dev)
> {
> int config_size = pci_config_size(pci_dev);
> @@ -660,6 +684,7 @@ static PCIDevice *do_pci_register_device(PCIDevice
> *pci_dev, PCIBus *bus,
> if (is_bridge) {
> pci_init_wmask_bridge(pci_dev);
> }
> + pci_init_multifunction(bus, pci_dev);
>
> if (!config_read)
> config_read = pci_default_read_config;
> --
> 1.6.6.1
