diff mbox series

[v5,3/5] i386/pc: pass pci_hole64_size to pc_memory_init()

Message ID 20220520104532.9816-4-joao.m.martins@oracle.com
State New
Headers show
Series [v5,1/5] hw/i386: add 4g boundary start to X86MachineState | expand

Commit Message

Joao Martins May 20, 2022, 10:45 a.m. UTC
Use the pre-initialized pci-host qdev and fetch the
pci-hole64-size into pc_memory_init() newly added argument.
piix needs a bit of care given all the !pci_enabled()
and that the pci_hole64_size is private to i440fx.

This is in preparation to determine that host-phys-bits are
enough and for pci-hole64-size to be considered to relocate
ram-above-4g to be at 1T (on AMD platforms).

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 hw/i386/pc.c                 | 3 ++-
 hw/i386/pc_piix.c            | 5 ++++-
 hw/i386/pc_q35.c             | 8 +++++++-
 hw/pci-host/i440fx.c         | 7 +++++++
 include/hw/i386/pc.h         | 3 ++-
 include/hw/pci-host/i440fx.h | 1 +
 6 files changed, 23 insertions(+), 4 deletions(-)

Comments

Igor Mammedov June 16, 2022, 1:30 p.m. UTC | #1
On Fri, 20 May 2022 11:45:30 +0100
Joao Martins <joao.m.martins@oracle.com> wrote:

> Use the pre-initialized pci-host qdev and fetch the
> pci-hole64-size into pc_memory_init() newly added argument.
> piix needs a bit of care given all the !pci_enabled()
> and that the pci_hole64_size is private to i440fx.
> 
> This is in preparation to determine that host-phys-bits are
> enough and for pci-hole64-size to be considered to relocate
> ram-above-4g to be at 1T (on AMD platforms).

modulo nit blow

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
>  hw/i386/pc.c                 | 3 ++-
>  hw/i386/pc_piix.c            | 5 ++++-
>  hw/i386/pc_q35.c             | 8 +++++++-
>  hw/pci-host/i440fx.c         | 7 +++++++
>  include/hw/i386/pc.h         | 3 ++-
>  include/hw/pci-host/i440fx.h | 1 +
>  6 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f7da1d5dd40d..af52d4ff89ef 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -799,7 +799,8 @@ void xen_load_linux(PCMachineState *pcms)
>  void pc_memory_init(PCMachineState *pcms,
>                      MemoryRegion *system_memory,
>                      MemoryRegion *rom_memory,
> -                    MemoryRegion **ram_memory)
> +                    MemoryRegion **ram_memory,
> +                    uint64_t pci_hole64_size)
>  {
>      int linux_boot, i;
>      MemoryRegion *option_rom_mr;
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 12d4a279c793..57bb5b8f2aea 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -91,6 +91,7 @@ static void pc_init1(MachineState *machine,
>      MemoryRegion *pci_memory;
>      MemoryRegion *rom_memory;
>      ram_addr_t lowmem;
> +    uint64_t hole64_size;

init it to 0 right here to avoid chance of run amok uninitialized variable?

>      DeviceState *i440fx_dev;
>  
>      /*
> @@ -166,10 +167,12 @@ static void pc_init1(MachineState *machine,
>          memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>          rom_memory = pci_memory;
>          i440fx_dev = qdev_new(host_type);
> +        hole64_size = i440fx_pci_hole64_size(i440fx_dev);
>      } else {
>          pci_memory = NULL;
>          rom_memory = system_memory;
>          i440fx_dev = NULL;
> +        hole64_size = 0;
>      }
>  
>      pc_guest_info_init(pcms);
> @@ -186,7 +189,7 @@ static void pc_init1(MachineState *machine,
>      /* allocate ram and load rom/bios */
>      if (!xen_enabled()) {
>          pc_memory_init(pcms, system_memory,
> -                       rom_memory, &ram_memory);
> +                       rom_memory, &ram_memory, hole64_size);
>      } else {
>          pc_system_flash_cleanup_unused(pcms);
>          if (machine->kernel_filename != NULL) {
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 8d867bdb274a..4d5c2fbd976b 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -138,6 +138,7 @@ static void pc_q35_init(MachineState *machine)
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      bool acpi_pcihp;
>      bool keep_pci_slot_hpc;
> +    uint64_t pci_hole64_size = 0;
>  
>      /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
>       * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
> @@ -206,8 +207,13 @@ static void pc_q35_init(MachineState *machine)
>      /* create pci host bus */
>      q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
>  
> +    if (pcmc->pci_enabled) {
> +        pci_hole64_size = q35_host->mch.pci_hole64_size;
> +    }
> +
>      /* allocate ram and load rom/bios */
> -    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory);
> +    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
> +                   pci_hole64_size);
>  
>      object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
>      object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index 5c1bab5c58ed..c5cc28250d5c 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -237,6 +237,13 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
>      }
>  }
>  
> +uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev)
> +{
> +        I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(i440fx_dev);
> +
> +        return i440fx->pci_hole64_size;
> +}
> +
>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                      DeviceState *dev,
>                      PCII440FXState **pi440fx_state,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ffcac5121ed9..9c847faea2f8 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -158,7 +158,8 @@ void xen_load_linux(PCMachineState *pcms);
>  void pc_memory_init(PCMachineState *pcms,
>                      MemoryRegion *system_memory,
>                      MemoryRegion *rom_memory,
> -                    MemoryRegion **ram_memory);
> +                    MemoryRegion **ram_memory,
> +                    uint64_t pci_hole64_size);
>  uint64_t pc_pci_hole64_start(void);
>  DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
>  void pc_basic_device_init(struct PCMachineState *pcms,
> diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
> index c4710445e30a..1299d6a2b0e4 100644
> --- a/include/hw/pci-host/i440fx.h
> +++ b/include/hw/pci-host/i440fx.h
> @@ -45,5 +45,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                      MemoryRegion *pci_memory,
>                      MemoryRegion *ram_memory);
>  
> +uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev);
>  
>  #endif
Michael S. Tsirkin June 16, 2022, 2:16 p.m. UTC | #2
On Thu, Jun 16, 2022 at 03:30:14PM +0200, Igor Mammedov wrote:
> On Fri, 20 May 2022 11:45:30 +0100
> Joao Martins <joao.m.martins@oracle.com> wrote:
> 
> > Use the pre-initialized pci-host qdev and fetch the
> > pci-hole64-size into pc_memory_init() newly added argument.
> > piix needs a bit of care given all the !pci_enabled()
> > and that the pci_hole64_size is private to i440fx.
> > 
> > This is in preparation to determine that host-phys-bits are
> > enough and for pci-hole64-size to be considered to relocate
> > ram-above-4g to be at 1T (on AMD platforms).
> 
> modulo nit blow
> 
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> 
> > 
> > Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> > ---
> >  hw/i386/pc.c                 | 3 ++-
> >  hw/i386/pc_piix.c            | 5 ++++-
> >  hw/i386/pc_q35.c             | 8 +++++++-
> >  hw/pci-host/i440fx.c         | 7 +++++++
> >  include/hw/i386/pc.h         | 3 ++-
> >  include/hw/pci-host/i440fx.h | 1 +
> >  6 files changed, 23 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index f7da1d5dd40d..af52d4ff89ef 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -799,7 +799,8 @@ void xen_load_linux(PCMachineState *pcms)
> >  void pc_memory_init(PCMachineState *pcms,
> >                      MemoryRegion *system_memory,
> >                      MemoryRegion *rom_memory,
> > -                    MemoryRegion **ram_memory)
> > +                    MemoryRegion **ram_memory,
> > +                    uint64_t pci_hole64_size)
> >  {
> >      int linux_boot, i;
> >      MemoryRegion *option_rom_mr;
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 12d4a279c793..57bb5b8f2aea 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -91,6 +91,7 @@ static void pc_init1(MachineState *machine,
> >      MemoryRegion *pci_memory;
> >      MemoryRegion *rom_memory;
> >      ram_addr_t lowmem;
> > +    uint64_t hole64_size;
> 
> init it to 0 right here to avoid chance of run amok uninitialized variable?


I don't see why we should, compilers seems to be pretty good about warning
about these things nowdays.

> >      DeviceState *i440fx_dev;
> >  
> >      /*
> > @@ -166,10 +167,12 @@ static void pc_init1(MachineState *machine,
> >          memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
> >          rom_memory = pci_memory;
> >          i440fx_dev = qdev_new(host_type);
> > +        hole64_size = i440fx_pci_hole64_size(i440fx_dev);
> >      } else {
> >          pci_memory = NULL;
> >          rom_memory = system_memory;
> >          i440fx_dev = NULL;
> > +        hole64_size = 0;
> >      }
> >  
> >      pc_guest_info_init(pcms);
> > @@ -186,7 +189,7 @@ static void pc_init1(MachineState *machine,
> >      /* allocate ram and load rom/bios */
> >      if (!xen_enabled()) {
> >          pc_memory_init(pcms, system_memory,
> > -                       rom_memory, &ram_memory);
> > +                       rom_memory, &ram_memory, hole64_size);
> >      } else {
> >          pc_system_flash_cleanup_unused(pcms);
> >          if (machine->kernel_filename != NULL) {
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 8d867bdb274a..4d5c2fbd976b 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -138,6 +138,7 @@ static void pc_q35_init(MachineState *machine)
> >      MachineClass *mc = MACHINE_GET_CLASS(machine);
> >      bool acpi_pcihp;
> >      bool keep_pci_slot_hpc;
> > +    uint64_t pci_hole64_size = 0;
> >  
> >      /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
> >       * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
> > @@ -206,8 +207,13 @@ static void pc_q35_init(MachineState *machine)
> >      /* create pci host bus */
> >      q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
> >  
> > +    if (pcmc->pci_enabled) {
> > +        pci_hole64_size = q35_host->mch.pci_hole64_size;
> > +    }
> > +
> >      /* allocate ram and load rom/bios */
> > -    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory);
> > +    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
> > +                   pci_hole64_size);
> >  
> >      object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
> >      object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
> > diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> > index 5c1bab5c58ed..c5cc28250d5c 100644
> > --- a/hw/pci-host/i440fx.c
> > +++ b/hw/pci-host/i440fx.c
> > @@ -237,6 +237,13 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
> >      }
> >  }
> >  
> > +uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev)
> > +{
> > +        I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(i440fx_dev);
> > +
> > +        return i440fx->pci_hole64_size;
> > +}
> > +
> >  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >                      DeviceState *dev,
> >                      PCII440FXState **pi440fx_state,
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index ffcac5121ed9..9c847faea2f8 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -158,7 +158,8 @@ void xen_load_linux(PCMachineState *pcms);
> >  void pc_memory_init(PCMachineState *pcms,
> >                      MemoryRegion *system_memory,
> >                      MemoryRegion *rom_memory,
> > -                    MemoryRegion **ram_memory);
> > +                    MemoryRegion **ram_memory,
> > +                    uint64_t pci_hole64_size);
> >  uint64_t pc_pci_hole64_start(void);
> >  DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
> >  void pc_basic_device_init(struct PCMachineState *pcms,
> > diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
> > index c4710445e30a..1299d6a2b0e4 100644
> > --- a/include/hw/pci-host/i440fx.h
> > +++ b/include/hw/pci-host/i440fx.h
> > @@ -45,5 +45,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >                      MemoryRegion *pci_memory,
> >                      MemoryRegion *ram_memory);
> >  
> > +uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev);
> >  
> >  #endif
Joao Martins June 17, 2022, 11:13 a.m. UTC | #3
On 6/16/22 14:30, Igor Mammedov wrote:
> On Fri, 20 May 2022 11:45:30 +0100
> Joao Martins <joao.m.martins@oracle.com> wrote:
> 
>> Use the pre-initialized pci-host qdev and fetch the
>> pci-hole64-size into pc_memory_init() newly added argument.
>> piix needs a bit of care given all the !pci_enabled()
>> and that the pci_hole64_size is private to i440fx.
>>
>> This is in preparation to determine that host-phys-bits are
>> enough and for pci-hole64-size to be considered to relocate
>> ram-above-4g to be at 1T (on AMD platforms).
> 
> modulo nit blow
> 
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> 

I haven't tackled the initialization nit below but I would assume
you agree with the rest of the patch. Let me know if I should still
add the Rb tag.

>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>>  hw/i386/pc.c                 | 3 ++-
>>  hw/i386/pc_piix.c            | 5 ++++-
>>  hw/i386/pc_q35.c             | 8 +++++++-
>>  hw/pci-host/i440fx.c         | 7 +++++++
>>  include/hw/i386/pc.h         | 3 ++-
>>  include/hw/pci-host/i440fx.h | 1 +
>>  6 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index f7da1d5dd40d..af52d4ff89ef 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -799,7 +799,8 @@ void xen_load_linux(PCMachineState *pcms)
>>  void pc_memory_init(PCMachineState *pcms,
>>                      MemoryRegion *system_memory,
>>                      MemoryRegion *rom_memory,
>> -                    MemoryRegion **ram_memory)
>> +                    MemoryRegion **ram_memory,
>> +                    uint64_t pci_hole64_size)
>>  {
>>      int linux_boot, i;
>>      MemoryRegion *option_rom_mr;
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 12d4a279c793..57bb5b8f2aea 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -91,6 +91,7 @@ static void pc_init1(MachineState *machine,
>>      MemoryRegion *pci_memory;
>>      MemoryRegion *rom_memory;
>>      ram_addr_t lowmem;
>> +    uint64_t hole64_size;
> 
> init it to 0 right here to avoid chance of run amok uninitialized variable?
> 
I haven't done this given that mst disagreed, plus the fact that the code style of
the function seems to place the NULL initialization mostly left to else conditional
clause. Part of the reason I haven't inited @i440fx_dev to NULL here as well (now
i440fx_host. The location we use hole64_size is also the same location we are using
@i440fx_host.

>>      DeviceState *i440fx_dev;
>>  
>>      /*
>> @@ -166,10 +167,12 @@ static void pc_init1(MachineState *machine,
>>          memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>>          rom_memory = pci_memory;
>>          i440fx_dev = qdev_new(host_type);
>> +        hole64_size = i440fx_pci_hole64_size(i440fx_dev);
>>      } else {
>>          pci_memory = NULL;
>>          rom_memory = system_memory;
>>          i440fx_dev = NULL;
>> +        hole64_size = 0;
>>      }
>>  
>>      pc_guest_info_init(pcms);
>> @@ -186,7 +189,7 @@ static void pc_init1(MachineState *machine,
>>      /* allocate ram and load rom/bios */
>>      if (!xen_enabled()) {
>>          pc_memory_init(pcms, system_memory,
>> -                       rom_memory, &ram_memory);
>> +                       rom_memory, &ram_memory, hole64_size);
>>      } else {
>>          pc_system_flash_cleanup_unused(pcms);
>>          if (machine->kernel_filename != NULL) {
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index 8d867bdb274a..4d5c2fbd976b 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -138,6 +138,7 @@ static void pc_q35_init(MachineState *machine)
>>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>>      bool acpi_pcihp;
>>      bool keep_pci_slot_hpc;
>> +    uint64_t pci_hole64_size = 0;
>>  
>>      /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
>>       * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
>> @@ -206,8 +207,13 @@ static void pc_q35_init(MachineState *machine)
>>      /* create pci host bus */
>>      q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
>>  
>> +    if (pcmc->pci_enabled) {
>> +        pci_hole64_size = q35_host->mch.pci_hole64_size;
>> +    }
>> +
>>      /* allocate ram and load rom/bios */
>> -    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory);
>> +    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
>> +                   pci_hole64_size);
>>  
>>      object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
>>      object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
>> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
>> index 5c1bab5c58ed..c5cc28250d5c 100644
>> --- a/hw/pci-host/i440fx.c
>> +++ b/hw/pci-host/i440fx.c
>> @@ -237,6 +237,13 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
>>      }
>>  }
>>  
>> +uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev)
>> +{
>> +        I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(i440fx_dev);
>> +
>> +        return i440fx->pci_hole64_size;
>> +}
>> +
>>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>                      DeviceState *dev,
>>                      PCII440FXState **pi440fx_state,
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index ffcac5121ed9..9c847faea2f8 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -158,7 +158,8 @@ void xen_load_linux(PCMachineState *pcms);
>>  void pc_memory_init(PCMachineState *pcms,
>>                      MemoryRegion *system_memory,
>>                      MemoryRegion *rom_memory,
>> -                    MemoryRegion **ram_memory);
>> +                    MemoryRegion **ram_memory,
>> +                    uint64_t pci_hole64_size);
>>  uint64_t pc_pci_hole64_start(void);
>>  DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
>>  void pc_basic_device_init(struct PCMachineState *pcms,
>> diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
>> index c4710445e30a..1299d6a2b0e4 100644
>> --- a/include/hw/pci-host/i440fx.h
>> +++ b/include/hw/pci-host/i440fx.h
>> @@ -45,5 +45,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>                      MemoryRegion *pci_memory,
>>                      MemoryRegion *ram_memory);
>>  
>> +uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev);
>>  
>>  #endif
>
Igor Mammedov June 17, 2022, 11:58 a.m. UTC | #4
On Fri, 17 Jun 2022 12:13:45 +0100
Joao Martins <joao.m.martins@oracle.com> wrote:

> On 6/16/22 14:30, Igor Mammedov wrote:
> > On Fri, 20 May 2022 11:45:30 +0100
> > Joao Martins <joao.m.martins@oracle.com> wrote:
> >   
> >> Use the pre-initialized pci-host qdev and fetch the
> >> pci-hole64-size into pc_memory_init() newly added argument.
> >> piix needs a bit of care given all the !pci_enabled()
> >> and that the pci_hole64_size is private to i440fx.
> >>
> >> This is in preparation to determine that host-phys-bits are
> >> enough and for pci-hole64-size to be considered to relocate
> >> ram-above-4g to be at 1T (on AMD platforms).  
> > 
> > modulo nit blow
> > 
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> >   
> 
> I haven't tackled the initialization nit below but I would assume
> you agree with the rest of the patch. Let me know if I should still
> add the Rb tag.

My ack still stands
 
> >>
> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> >> ---
> >>  hw/i386/pc.c                 | 3 ++-
> >>  hw/i386/pc_piix.c            | 5 ++++-
> >>  hw/i386/pc_q35.c             | 8 +++++++-
> >>  hw/pci-host/i440fx.c         | 7 +++++++
> >>  include/hw/i386/pc.h         | 3 ++-
> >>  include/hw/pci-host/i440fx.h | 1 +
> >>  6 files changed, 23 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index f7da1d5dd40d..af52d4ff89ef 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -799,7 +799,8 @@ void xen_load_linux(PCMachineState *pcms)
> >>  void pc_memory_init(PCMachineState *pcms,
> >>                      MemoryRegion *system_memory,
> >>                      MemoryRegion *rom_memory,
> >> -                    MemoryRegion **ram_memory)
> >> +                    MemoryRegion **ram_memory,
> >> +                    uint64_t pci_hole64_size)
> >>  {
> >>      int linux_boot, i;
> >>      MemoryRegion *option_rom_mr;
> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> index 12d4a279c793..57bb5b8f2aea 100644
> >> --- a/hw/i386/pc_piix.c
> >> +++ b/hw/i386/pc_piix.c
> >> @@ -91,6 +91,7 @@ static void pc_init1(MachineState *machine,
> >>      MemoryRegion *pci_memory;
> >>      MemoryRegion *rom_memory;
> >>      ram_addr_t lowmem;
> >> +    uint64_t hole64_size;  
> > 
> > init it to 0 right here to avoid chance of run amok uninitialized variable?
> >   
> I haven't done this given that mst disagreed, plus the fact that the code style of
> the function seems to place the NULL initialization mostly left to else conditional
> clause. Part of the reason I haven't inited @i440fx_dev to NULL here as well (now
> i440fx_host. The location we use hole64_size is also the same location we are using
> @i440fx_host.
> 
> >>      DeviceState *i440fx_dev;
> >>  
> >>      /*
> >> @@ -166,10 +167,12 @@ static void pc_init1(MachineState *machine,
> >>          memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
> >>          rom_memory = pci_memory;
> >>          i440fx_dev = qdev_new(host_type);
> >> +        hole64_size = i440fx_pci_hole64_size(i440fx_dev);
> >>      } else {
> >>          pci_memory = NULL;
> >>          rom_memory = system_memory;
> >>          i440fx_dev = NULL;
> >> +        hole64_size = 0;
> >>      }
> >>  
> >>      pc_guest_info_init(pcms);
> >> @@ -186,7 +189,7 @@ static void pc_init1(MachineState *machine,
> >>      /* allocate ram and load rom/bios */
> >>      if (!xen_enabled()) {
> >>          pc_memory_init(pcms, system_memory,
> >> -                       rom_memory, &ram_memory);
> >> +                       rom_memory, &ram_memory, hole64_size);
> >>      } else {
> >>          pc_system_flash_cleanup_unused(pcms);
> >>          if (machine->kernel_filename != NULL) {
> >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> >> index 8d867bdb274a..4d5c2fbd976b 100644
> >> --- a/hw/i386/pc_q35.c
> >> +++ b/hw/i386/pc_q35.c
> >> @@ -138,6 +138,7 @@ static void pc_q35_init(MachineState *machine)
> >>      MachineClass *mc = MACHINE_GET_CLASS(machine);
> >>      bool acpi_pcihp;
> >>      bool keep_pci_slot_hpc;
> >> +    uint64_t pci_hole64_size = 0;
> >>  
> >>      /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
> >>       * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
> >> @@ -206,8 +207,13 @@ static void pc_q35_init(MachineState *machine)
> >>      /* create pci host bus */
> >>      q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
> >>  
> >> +    if (pcmc->pci_enabled) {
> >> +        pci_hole64_size = q35_host->mch.pci_hole64_size;
> >> +    }
> >> +
> >>      /* allocate ram and load rom/bios */
> >> -    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory);
> >> +    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
> >> +                   pci_hole64_size);
> >>  
> >>      object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
> >>      object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
> >> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> >> index 5c1bab5c58ed..c5cc28250d5c 100644
> >> --- a/hw/pci-host/i440fx.c
> >> +++ b/hw/pci-host/i440fx.c
> >> @@ -237,6 +237,13 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
> >>      }
> >>  }
> >>  
> >> +uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev)
> >> +{
> >> +        I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(i440fx_dev);
> >> +
> >> +        return i440fx->pci_hole64_size;
> >> +}
> >> +
> >>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>                      DeviceState *dev,
> >>                      PCII440FXState **pi440fx_state,
> >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >> index ffcac5121ed9..9c847faea2f8 100644
> >> --- a/include/hw/i386/pc.h
> >> +++ b/include/hw/i386/pc.h
> >> @@ -158,7 +158,8 @@ void xen_load_linux(PCMachineState *pcms);
> >>  void pc_memory_init(PCMachineState *pcms,
> >>                      MemoryRegion *system_memory,
> >>                      MemoryRegion *rom_memory,
> >> -                    MemoryRegion **ram_memory);
> >> +                    MemoryRegion **ram_memory,
> >> +                    uint64_t pci_hole64_size);
> >>  uint64_t pc_pci_hole64_start(void);
> >>  DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
> >>  void pc_basic_device_init(struct PCMachineState *pcms,
> >> diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
> >> index c4710445e30a..1299d6a2b0e4 100644
> >> --- a/include/hw/pci-host/i440fx.h
> >> +++ b/include/hw/pci-host/i440fx.h
> >> @@ -45,5 +45,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>                      MemoryRegion *pci_memory,
> >>                      MemoryRegion *ram_memory);
> >>  
> >> +uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev);
> >>  
> >>  #endif  
> >   
>
diff mbox series

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f7da1d5dd40d..af52d4ff89ef 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -799,7 +799,8 @@  void xen_load_linux(PCMachineState *pcms)
 void pc_memory_init(PCMachineState *pcms,
                     MemoryRegion *system_memory,
                     MemoryRegion *rom_memory,
-                    MemoryRegion **ram_memory)
+                    MemoryRegion **ram_memory,
+                    uint64_t pci_hole64_size)
 {
     int linux_boot, i;
     MemoryRegion *option_rom_mr;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 12d4a279c793..57bb5b8f2aea 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -91,6 +91,7 @@  static void pc_init1(MachineState *machine,
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
     ram_addr_t lowmem;
+    uint64_t hole64_size;
     DeviceState *i440fx_dev;
 
     /*
@@ -166,10 +167,12 @@  static void pc_init1(MachineState *machine,
         memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
         i440fx_dev = qdev_new(host_type);
+        hole64_size = i440fx_pci_hole64_size(i440fx_dev);
     } else {
         pci_memory = NULL;
         rom_memory = system_memory;
         i440fx_dev = NULL;
+        hole64_size = 0;
     }
 
     pc_guest_info_init(pcms);
@@ -186,7 +189,7 @@  static void pc_init1(MachineState *machine,
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
         pc_memory_init(pcms, system_memory,
-                       rom_memory, &ram_memory);
+                       rom_memory, &ram_memory, hole64_size);
     } else {
         pc_system_flash_cleanup_unused(pcms);
         if (machine->kernel_filename != NULL) {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 8d867bdb274a..4d5c2fbd976b 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -138,6 +138,7 @@  static void pc_q35_init(MachineState *machine)
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     bool acpi_pcihp;
     bool keep_pci_slot_hpc;
+    uint64_t pci_hole64_size = 0;
 
     /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
      * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
@@ -206,8 +207,13 @@  static void pc_q35_init(MachineState *machine)
     /* create pci host bus */
     q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
 
+    if (pcmc->pci_enabled) {
+        pci_hole64_size = q35_host->mch.pci_hole64_size;
+    }
+
     /* allocate ram and load rom/bios */
-    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory);
+    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
+                   pci_hole64_size);
 
     object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
     object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 5c1bab5c58ed..c5cc28250d5c 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -237,6 +237,13 @@  static void i440fx_realize(PCIDevice *dev, Error **errp)
     }
 }
 
+uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev)
+{
+        I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(i440fx_dev);
+
+        return i440fx->pci_hole64_size;
+}
+
 PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     DeviceState *dev,
                     PCII440FXState **pi440fx_state,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ffcac5121ed9..9c847faea2f8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -158,7 +158,8 @@  void xen_load_linux(PCMachineState *pcms);
 void pc_memory_init(PCMachineState *pcms,
                     MemoryRegion *system_memory,
                     MemoryRegion *rom_memory,
-                    MemoryRegion **ram_memory);
+                    MemoryRegion **ram_memory,
+                    uint64_t pci_hole64_size);
 uint64_t pc_pci_hole64_start(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(struct PCMachineState *pcms,
diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
index c4710445e30a..1299d6a2b0e4 100644
--- a/include/hw/pci-host/i440fx.h
+++ b/include/hw/pci-host/i440fx.h
@@ -45,5 +45,6 @@  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     MemoryRegion *pci_memory,
                     MemoryRegion *ram_memory);
 
+uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev);
 
 #endif