On 02.12.21 18:11, Matthew Rosato wrote:
> 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 <[email protected]>
>>> ---
>>> 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?
>
Good question, I'm certainly not a zPCI expert. However if you think
that we cannot really break migration in a sane use case, I'm fine with
it as well :)
The question about breaking migration just popped up in my head when I
stumbled over this patch.
--
Thanks,
David / dhildenb
