Re: [PULL v3 26/32] s390x/pci: use a PCI Group structure

[Philippe Mathieu-Daudé]

On 11/17/20 1:06 PM, Cornelia Huck wrote:
> On Tue, 17 Nov 2020 12:55:22 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> On 11/17/20 12:43 PM, Cornelia Huck wrote:
>>> On Sun, 01 Nov 2020 14:02:46 -0700
>>> Alex Williamson <alex.williamson@redhat.com> wrote:
>>>   
>>>> From: Pierre Morel <pmorel@linux.ibm.com>
>>>>
>>>> We use a S390PCIGroup structure to hold the information related to a
>>>> zPCI Function group.
>>>>
>>>> This allows us to be ready to support multiple groups and to retrieve
>>>> the group information from the host.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>>>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>>>> ---
>>>>  hw/s390x/s390-pci-bus.c         |   42 
>>>> +++++++++++++++++++++++++++++++++++++++
>>>>  hw/s390x/s390-pci-inst.c        |   23 +++++++++++++--------
>>>>  include/hw/s390x/s390-pci-bus.h |   10 +++++++++
>>>>  3 files changed, 66 insertions(+), 9 deletions(-)  
>>>
>>> I just bisected a regression down to this commit.
>>>
>>> s390x tcg guest on x86, virtio-pci devices are not detected. The
>>> relevant feature bits are visible to the guest. Same breakage with
>>> different guest kernels.
>>>
>>> KVM guests and s390x tcg guests on s390x are fine, so I assume an
>>> endianness issue somewhere. Nothing jumps out to me, though.
> 
> (...)
>>>>      case CLP_QUERY_PCI_FNGRP: {
>>>>          ClpRspQueryPciGrp *resgrp = (ClpRspQueryPciGrp *)resh;
>>>> -        resgrp->fr = 1;
>>>> -        stq_p(&resgrp->dasm, 0);
>>>> -        stq_p(&resgrp->msia, ZPCI_MSI_ADDR);
>>>> -        stw_p(&resgrp->mui, DEFAULT_MUI);
>>>> -        stw_p(&resgrp->i, 128);
>>>> -        stw_p(&resgrp->maxstbl, 128);
>>>> -        resgrp->version = 0;
>>>>  
>>>> +        ClpReqQueryPciGrp *reqgrp = (ClpReqQueryPciGrp *)reqh;
>>>> +        S390PCIGroup *group;
>>>> +
>>>> +        group = s390_group_find(reqgrp->g);  
>>
>> -        group = s390_group_find(reqgrp->g);
>> +        group = s390_group_find(ldl_p(&reqgrp->g));
> 
> Yep, that's it :)
> 
> Do you want to send a proper patch, or should I do it?

Sent 5min ago ;)

Regards,

Phil.