pci-host: Allow extended config space access for PowerNV PHB4 model

[PATCH] pci-host: Allow extended config space access for PowerNV PHB4 model

Posted by Christophe Lombard 2 years, 6 months ago

The PCIe extended configuration space on the device is not currently
accessible to the host. if by default,  it is still inaccessible for
conventional for PCIe buses, add the current flag
PCI_BUS_EXTENDED_CONFIG_SPACE on the root bus permits PCI-E extended
config space access.

Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
 hw/pci-host/pnv_phb4.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 5c375a9f28..40b793201a 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1205,6 +1205,7 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
                                      &phb->pci_mmio, &phb->pci_io,
                                      0, 4, TYPE_PNV_PHB4_ROOT_BUS);
     pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
+    pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
 
     /* Add a single Root port */
     qdev_prop_set_uint8(DEVICE(&phb->root), "chassis", phb->chip_id);
-- 
2.33.1

Re: [PATCH] pci-host: Allow extended config space access for PowerNV PHB4 model

Posted by Cédric Le Goater 2 years, 6 months ago

On 11/9/21 15:50, Christophe Lombard wrote:
> The PCIe extended configuration space on the device is not currently
> accessible to the host. if by default,  it is still inaccessible for
> conventional for PCIe buses, add the current flag
> PCI_BUS_EXTENDED_CONFIG_SPACE on the root bus permits PCI-E extended
> config space access.
> 
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> ---
>   hw/pci-host/pnv_phb4.c | 1 +
>   1 file changed, 1 insertion(+)

Queued for 7.0.

Thanks,

C.

Re: [PATCH] pci-host: Allow extended config space access for PowerNV PHB4 model

Posted by Frederic Barrat 2 years, 6 months ago


On 09/11/2021 15:50, Christophe Lombard wrote:
> The PCIe extended configuration space on the device is not currently
> accessible to the host. if by default,  it is still inaccessible for
> conventional for PCIe buses, add the current flag
> PCI_BUS_EXTENDED_CONFIG_SPACE on the root bus permits PCI-E extended
> config space access.
> 
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> ---


FWIW, looks good to me
Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>



>   hw/pci-host/pnv_phb4.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 5c375a9f28..40b793201a 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1205,6 +1205,7 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>                                        &phb->pci_mmio, &phb->pci_io,
>                                        0, 4, TYPE_PNV_PHB4_ROOT_BUS);
>       pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
> +    pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
>   
>       /* Add a single Root port */
>       qdev_prop_set_uint8(DEVICE(&phb->root), "chassis", phb->chip_id);
>

Re: [PATCH] pci-host: Allow extended config space access for PowerNV PHB4 model

Posted by Cédric Le Goater 2 years, 6 months ago

On 11/9/21 16:51, Frederic Barrat wrote:
> 
> 
> On 09/11/2021 15:50, Christophe Lombard wrote:
>> The PCIe extended configuration space on the device is not currently
>> accessible to the host. if by default,  it is still inaccessible for
>> conventional for PCIe buses, add the current flag
>> PCI_BUS_EXTENDED_CONFIG_SPACE on the root bus permits PCI-E extended
>> config space access.

For the record, this is coming from an experiment of plugging a
CXL device on a QEMU PowerNV POWER10 machine (baremetal). Only
minor changes (64 bits ops) were required to get it working.
  
I wonder where are with the CXL models ?

>> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
>> ---
> 
> 
> FWIW, looks good to me
> Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>



Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> 
> 
> 
>>   hw/pci-host/pnv_phb4.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>> index 5c375a9f28..40b793201a 100644
>> --- a/hw/pci-host/pnv_phb4.c
>> +++ b/hw/pci-host/pnv_phb4.c
>> @@ -1205,6 +1205,7 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>>                                        &phb->pci_mmio, &phb->pci_io,
>>                                        0, 4, TYPE_PNV_PHB4_ROOT_BUS);
>>       pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
>> +    pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
>>       /* Add a single Root port */
>>       qdev_prop_set_uint8(DEVICE(&phb->root), "chassis", phb->chip_id);
>>

Re: [PATCH] pci-host: Allow extended config space access for PowerNV PHB4 model

Posted by Philippe Mathieu-Daudé 2 years, 5 months ago

On 11/9/21 17:04, Cédric Le Goater wrote:
> On 11/9/21 16:51, Frederic Barrat wrote:
>>
>>
>> On 09/11/2021 15:50, Christophe Lombard wrote:
>>> The PCIe extended configuration space on the device is not currently
>>> accessible to the host. if by default,  it is still inaccessible for
>>> conventional for PCIe buses, add the current flag
>>> PCI_BUS_EXTENDED_CONFIG_SPACE on the root bus permits PCI-E extended
>>> config space access.
> 
> For the record, this is coming from an experiment of plugging a
> CXL device on a QEMU PowerNV POWER10 machine (baremetal). Only
> minor changes (64 bits ops) were required to get it working.

Since this note could be helpful when having future retrospective,
do you mind amending this note to the commit description?

> I wonder where are with the CXL models ?

IIRC Ben worked actively, asked help to the community but received
very few, basically because there is not enough man power IMHO.

Last thing I remember is Igor suggested a different design approach:
https://lore.kernel.org/qemu-devel/20210319180705.6ede9091@redhat.com/

>>> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
>>> ---
>>
>>
>> FWIW, looks good to me
>> Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
> 
> 
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> Thanks,
> 
> C.
> 
> 
>>
>>
>>
>>>   hw/pci-host/pnv_phb4.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>>> index 5c375a9f28..40b793201a 100644
>>> --- a/hw/pci-host/pnv_phb4.c
>>> +++ b/hw/pci-host/pnv_phb4.c
>>> @@ -1205,6 +1205,7 @@ static void pnv_phb4_realize(DeviceState *dev,
>>> Error **errp)
>>>                                        &phb->pci_mmio, &phb->pci_io,
>>>                                        0, 4, TYPE_PNV_PHB4_ROOT_BUS);
>>>       pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
>>> +    pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
>>>       /* Add a single Root port */
>>>       qdev_prop_set_uint8(DEVICE(&phb->root), "chassis", phb->chip_id);
>>>
> 
>

Re: [PATCH] pci-host: Allow extended config space access for PowerNV PHB4 model

Posted by Cédric Le Goater 2 years, 5 months ago

On 11/17/21 08:59, Philippe Mathieu-Daudé wrote:
> On 11/9/21 17:04, Cédric Le Goater wrote:
>> On 11/9/21 16:51, Frederic Barrat wrote:
>>>
>>>
>>> On 09/11/2021 15:50, Christophe Lombard wrote:
>>>> The PCIe extended configuration space on the device is not currently
>>>> accessible to the host. if by default,  it is still inaccessible for
>>>> conventional for PCIe buses, add the current flag
>>>> PCI_BUS_EXTENDED_CONFIG_SPACE on the root bus permits PCI-E extended
>>>> config space access.
>>
>> For the record, this is coming from an experiment of plugging a
>> CXL device on a QEMU PowerNV POWER10 machine (baremetal). Only
>> minor changes (64 bits ops) were required to get it working.
> 
> Since this note could be helpful when having future retrospective,
> do you mind amending this note to the commit description?

Yes. I agree. Please do.
  
>> I wonder where we are with the CXL models ?
> 
> IIRC Ben worked actively, asked help to the community but received
> very few, basically because there is not enough man power IMHO.
> 
> Last thing I remember is Igor suggested a different design approach:
> https://lore.kernel.org/qemu-devel/20210319180705.6ede9091@redhat.com/

Well, the CXL Linux driver seemed to work fine on QEMU machines, Intel
and POWER.

Thanks for the info,

C.