Re: [Qemu-devel] [PATCH v6 2/2] enable multi-function hot-add

[Cao jin]

Hi Alex

On 10/27/2015 10:21 PM, Alex Williamson wrote:
> On Tue, 2015-10-27 at 20:51 +0800, Cao jin wrote:
> > Enable PCIe device multi-function hot-add, just ensure function 0 is added
> > last, then driver will got the notification to scan the slot.
> >
> > Signed-off-by: Cao jin <address@hidden>
> > ---
> >   hw/pci/pci.c         | 40 +++++++++++++++++++++++++++++++++++++++-
> >   hw/pci/pci_host.c    | 13 +++++++++++--
> >   hw/pci/pcie.c        | 18 +++++++++---------
> >   include/hw/pci/pci.h |  1 +
> >   4 files changed, 60 insertions(+), 12 deletions(-)
>
> > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> > index 3e26f92..63d7d2f 100644
> > --- a/hw/pci/pci_host.c
> > +++ b/hw/pci/pci_host.c
> > @@ -20,6 +20,7 @@
> >
> >   #include "hw/pci/pci.h"
> >   #include "hw/pci/pci_host.h"
> > +#include "hw/pci/pci_bus.h"
> >   #include "trace.h"
> >
> >   /* debug PCI */
> > @@ -75,7 +76,11 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, 
> > int len)
> >       PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
> >       uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
> >
> > -    if (!pci_dev) {
> > +    /* non-zero functions are only exposed when function 0 is present,
> > +     * allowing direct removal of unexposed functions.
> > +     */
> > +    if (!pci_dev ||
> > +        (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev))) {
> >           return;
> >       }
> >
> > @@ -91,7 +96,11 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
> >       uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
> >       uint32_t val;
> >
> > -    if (!pci_dev) {
> > +    /* non-zero functions are only exposed when function 0 is present,
> > +     * allowing direct removal of unexposed functions.
> > +     */
> > +    if (!pci_dev ||
> > +        (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev))) {
> >           return ~0x0;
> >       }
>
>
> Don't we need to do this in pci_host_config_read_common() and
> pci_host_config_write_common() instead?  Otherwise I don't think we
> catch config access via mmcfg.  Thanks,

Yup...my carelessness, will fix it. Thanks for reminding:)

> Alex
>
> .

--
Yours Sincerely,

Cao Jin