Re: [Qemu-devel] [Qemu-ppc] [PATCH v3] pci: allow 0 address for PCI IO/MEM regions

[Laurent Vivier <lvivier@redhat.com>]

Ping ?

On 27/07/2015 10:19, Laurent Vivier wrote:
> Alex,
> 
> could you ACK this patch ?
> 
> It's not perfect and it will be removed later, but for the moment it
> allows to hotplug PCI card in pseries.
> 
> Laurent
> 
> On 24/07/2015 10:35, Laurent Vivier wrote:
>> Some kernels program a 0 address for io regions. PCI 3.0 spec
>> section 6.2.5.1 doesn't seem to disallow this.
>>
>> based on patch by Michael Roth <mdroth@linux.vnet.ibm.com>
>>
>> Add pci_allow_0_addr in MachineClass to conditionally
>> allow addr 0 for pseries, as this can break other architectures.
>>
>> This patch allows to hotplug PCI card in pseries machine, as the first
>> added card BAR0 is always set to 0 address.
>>
>> This as a temporary hack, waiting to fix PCI memory priorities for more
>> machine types...
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> v2: move flag from PCIBus to MachineClass, rename it pci_allow_0_address
>> v3: change author, update commit message.
>>  hw/pci/pci.c        | 12 +++++++++---
>>  hw/ppc/spapr.c      |  1 +
>>  include/hw/boards.h |  3 ++-
>>  3 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index a017614..9f57aea 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -38,6 +38,7 @@
>>  #include "hw/pci/msix.h"
>>  #include "exec/address-spaces.h"
>>  #include "hw/hotplug.h"
>> +#include "hw/boards.h"
>>  
>>  //#define DEBUG_PCI
>>  #ifdef DEBUG_PCI
>> @@ -1065,6 +1066,10 @@ static pcibus_t pci_bar_address(PCIDevice *d,
>>      pcibus_t new_addr, last_addr;
>>      int bar = pci_bar(d, reg);
>>      uint16_t cmd = pci_get_word(d->config + PCI_COMMAND);
>> +    Object *machine = qdev_get_machine();
>> +    ObjectClass *oc = object_get_class(machine);
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +    bool allow_0_address = mc->pci_allow_0_address;
>>  
>>      if (type & PCI_BASE_ADDRESS_SPACE_IO) {
>>          if (!(cmd & PCI_COMMAND_IO)) {
>> @@ -1075,7 +1080,8 @@ static pcibus_t pci_bar_address(PCIDevice *d,
>>          /* Check if 32 bit BAR wraps around explicitly.
>>           * TODO: make priorities correct and remove this work around.
>>           */
>> -        if (last_addr <= new_addr || new_addr == 0 || last_addr >= UINT32_MAX) {
>> +        if (last_addr <= new_addr || last_addr >= UINT32_MAX ||
>> +            (!allow_0_address && new_addr == 0)) {
>>              return PCI_BAR_UNMAPPED;
>>          }
>>          return new_addr;
>> @@ -1099,8 +1105,8 @@ static pcibus_t pci_bar_address(PCIDevice *d,
>>      /* XXX: as we cannot support really dynamic
>>         mappings, we handle specific values as invalid
>>         mappings. */
>> -    if (last_addr <= new_addr || new_addr == 0 ||
>> -        last_addr == PCI_BAR_UNMAPPED) {
>> +    if (last_addr <= new_addr || last_addr == PCI_BAR_UNMAPPED ||
>> +        (!allow_0_address && new_addr == 0)) {
>>          return PCI_BAR_UNMAPPED;
>>      }
>>  
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index a6f1947..bf0c64f 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1835,6 +1835,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>      mc->default_ram_size = 512 * M_BYTE;
>>      mc->kvm_type = spapr_kvm_type;
>>      mc->has_dynamic_sysbus = true;
>> +    mc->pci_allow_0_address = true;
>>  
>>      fwc->get_dev_path = spapr_get_fw_dev_path;
>>      nc->nmi_monitor_handler = spapr_nmi;
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 2aec9cb..3f84afd 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -100,7 +100,8 @@ struct MachineClass {
>>          no_cdrom:1,
>>          no_sdcard:1,
>>          has_dynamic_sysbus:1,
>> -        no_tco:1;
>> +        no_tco:1,
>> +        pci_allow_0_address:1;
>>      int is_default;
>>      const char *default_machine_opts;
>>      const char *default_boot_order;
>>
>