Re: [PATCH 1/4] s390x/pci: use a reserved ID for the default PCI group

[Matthew Rosato]

On 12/2/21 11:43 AM, David Hildenbrand wrote:
> On 02.12.21 17:41, Matthew Rosato wrote:
> > The current default PCI group being used can technically collide with a
> > real group ID passed from a hostdev.  Let's instead use a group ID that comes
> > from a special pool that is architected to be reserved for simulated devices.
> >
> > Fixes: 28dc86a072 ("s390x/pci: use a PCI Group structure")
> > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> > ---
> >   include/hw/s390x/s390-pci-bus.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/hw/s390x/s390-pci-bus.h b/include/hw/s390x/s390-pci-bus.h
> > index aa891c178d..2727e7bdef 100644
> > --- a/include/hw/s390x/s390-pci-bus.h
> > +++ b/include/hw/s390x/s390-pci-bus.h
> > @@ -313,7 +313,7 @@ typedef struct ZpciFmb {
> >   } ZpciFmb;
> >   QEMU_BUILD_BUG_MSG(offsetof(ZpciFmb, fmt0) != 48, "padding in ZpciFmb");
> >   
> > -#define ZPCI_DEFAULT_FN_GRP 0x20
> > +#define ZPCI_DEFAULT_FN_GRP 0xFF
> >   typedef struct S390PCIGroup {
> >       ClpRspQueryPciGrp zpci_group;
> >       int id;
>
> What happens if we migrate a VM from old to new QEMU? Won't the guest be
> able to observe the change?

Yes, technically --  But # itself is not really all that important, it 
is provided from CLP Q PCI FN to be subsequently used as input into Q 
PCI FNGRP -- With the fundamental notion being that all functions that 
share the same group # share the same group CLP info.  Whether the 
number is, say, 1 or 5 doesn't matter so much.

However..  0xF0 and greater are the only values reserved for hypervisor 
use.  By using 0x20 we run the risk of accidentally conflating simulated 
devices and real hardware, hence the desire to change it.

Is your concern about a migrated guest with a virtio device trying to do 
a CLP QUERY PCI FNGRP using 0x20 on a new QEMU?  I suppose we could 
modify 'clp_service_call, case CLP_QUERY_PCI_FNGRP' to silently catch 
simulated devices trying to use something other than the default group, 
e.g.:

if ((pbdev->fh & FH_SHM_EMUL) &&
    (pbdev->zpci_fn.pfgid != ZPCI_DEFAULT_FN_GRP)) {
        /* Simulated device MUST have default group */
        pbdev->zpci_fn.pfgid = ZPCI_DEFAULT_FN_GRP;
        group = s390_group_find(ZPCI_DEFAULT_FN_GRP);
}

What do you think?