[v6,16/18] hw/arm/virt: Add nvdimm hot-plug infrastructure
diff mbox series

Message ID 20190205173306.20483-17-eric.auger@redhat.com
State New
Headers show
Series
  • ARM virt: Initial RAM expansion and PCDIMM/NVDIMM support
Related show

Commit Message

Auger Eric Feb. 5, 2019, 5:33 p.m. UTC
From: Kwangwoo Lee <kwangwoo.lee@sk.com>

Pre-plug and plug handlers are prepared for NVDIMM support.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Kwangwoo Lee <kwangwoo.lee@sk.com>
---
 default-configs/arm-softmmu.mak |  2 ++
 hw/arm/virt-acpi-build.c        |  6 ++++++
 hw/arm/virt.c                   | 22 ++++++++++++++++++++++
 include/hw/arm/virt.h           |  3 +++
 4 files changed, 33 insertions(+)

Comments

Igor Mammedov Feb. 18, 2019, 10:30 a.m. UTC | #1
On Tue,  5 Feb 2019 18:33:04 +0100
Eric Auger <eric.auger@redhat.com> wrote:

> From: Kwangwoo Lee <kwangwoo.lee@sk.com>
> 
> Pre-plug and plug handlers are prepared for NVDIMM support.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Kwangwoo Lee <kwangwoo.lee@sk.com>
> ---
>  default-configs/arm-softmmu.mak |  2 ++
>  hw/arm/virt-acpi-build.c        |  6 ++++++
>  hw/arm/virt.c                   | 22 ++++++++++++++++++++++
>  include/hw/arm/virt.h           |  3 +++
>  4 files changed, 33 insertions(+)
> 
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index dc4624794f..ddbe87ed15 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -162,3 +162,5 @@ CONFIG_HIGHBANK=y
>  CONFIG_MUSICPAL=y
>  CONFIG_MEM_DEVICE=y
>  CONFIG_DIMM=y
> +CONFIG_NVDIMM=y
> +CONFIG_ACPI_NVDIMM=y
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 781eafaf5e..f086adfa82 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -784,6 +784,7 @@ static
>  void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>  {
>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> +    MachineState *ms = MACHINE(vms);
>      GArray *table_offsets;
>      unsigned dsdt, xsdt;
>      GArray *tables_blob = tables->table_data;
> @@ -824,6 +825,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>          }
>      }
>  
> +    if (vms->acpi_nvdimm_state.is_enabled) {
> +        nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
> +                          &vms->acpi_nvdimm_state, ms->ram_slots);
> +    }
> +
>      if (its_class_name() && !vmc->no_its) {
>          acpi_add_table(table_offsets, tables_blob);
>          build_iort(tables_blob, tables->linker, vms);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b683902991..0c8c2cc191 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -132,6 +132,7 @@ static const MemMapEntry a15memmap[] = {
>      [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
>      [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
>      [VIRT_SMMU] =               { 0x09050000, 0x00020000 },
> +    [VIRT_ACPI_IO] =            { 0x09070000, 0x00010000 },
where does this range come from and is its size sufficient?

>      [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>      [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
> @@ -1637,6 +1638,18 @@ static void machvirt_init(MachineState *machine)
>  
>      create_platform_bus(vms, pic);
>  
> +    if (vms->acpi_nvdimm_state.is_enabled) {
> +        AcpiNVDIMMState *acpi_nvdimm_state = &vms->acpi_nvdimm_state;
> +
> +        acpi_nvdimm_state->dsm_io.type = NVDIMM_ACPI_IO_MEMORY;
> +        acpi_nvdimm_state->dsm_io.base =
> +                vms->memmap[VIRT_ACPI_IO].base + NVDIMM_ACPI_IO_BASE;
> +        acpi_nvdimm_state->dsm_io.len = NVDIMM_ACPI_IO_LEN;
> +
> +        nvdimm_init_acpi_state(acpi_nvdimm_state, sysmem,
> +                               vms->fw_cfg, OBJECT(vms));
> +    }
> +
>      vms->bootinfo.ram_size = machine->ram_size;
>      vms->bootinfo.kernel_filename = machine->kernel_filename;
>      vms->bootinfo.kernel_cmdline = machine->kernel_cmdline;
> @@ -1822,10 +1835,19 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
>                               DeviceState *dev, Error **errp)
>  {
>      VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> +    bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
>      Error *local_err = NULL;
>  
>      pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err);
> +    if (local_err) {
> +        goto out;
> +    }
>  
> +    if (is_nvdimm) {
> +        nvdimm_plug(&vms->acpi_nvdimm_state);
> +    }
> +
> +out:
>      error_propagate(errp, local_err);
>  }
>  
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index c88f67a492..56d73b0e86 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -37,6 +37,7 @@
>  #include "hw/arm/arm.h"
>  #include "sysemu/kvm.h"
>  #include "hw/intc/arm_gicv3_common.h"
> +#include "hw/mem/nvdimm.h"
>  
>  #define NUM_GICV2M_SPIS       64
>  #define NUM_VIRTIO_TRANSPORTS 32
> @@ -77,6 +78,7 @@ enum {
>      VIRT_GPIO,
>      VIRT_SECURE_UART,
>      VIRT_SECURE_MEM,
> +    VIRT_ACPI_IO,
>      VIRT_LOWMEMMAP_LAST,
>  };
>  
> @@ -134,6 +136,7 @@ typedef struct {
>      hwaddr high_io_base;
>      hwaddr highest_gpa;
>      bool extended_memmap;
> +    AcpiNVDIMMState acpi_nvdimm_state;
>  } VirtMachineState;
>  
>  #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
Auger Eric Feb. 20, 2019, 3:21 p.m. UTC | #2
Hi Igor,

On 2/18/19 11:30 AM, Igor Mammedov wrote:
> On Tue,  5 Feb 2019 18:33:04 +0100
> Eric Auger <eric.auger@redhat.com> wrote:
> 
>> From: Kwangwoo Lee <kwangwoo.lee@sk.com>
>>
>> Pre-plug and plug handlers are prepared for NVDIMM support.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Kwangwoo Lee <kwangwoo.lee@sk.com>
>> ---
>>  default-configs/arm-softmmu.mak |  2 ++
>>  hw/arm/virt-acpi-build.c        |  6 ++++++
>>  hw/arm/virt.c                   | 22 ++++++++++++++++++++++
>>  include/hw/arm/virt.h           |  3 +++
>>  4 files changed, 33 insertions(+)
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index dc4624794f..ddbe87ed15 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -162,3 +162,5 @@ CONFIG_HIGHBANK=y
>>  CONFIG_MUSICPAL=y
>>  CONFIG_MEM_DEVICE=y
>>  CONFIG_DIMM=y
>> +CONFIG_NVDIMM=y
>> +CONFIG_ACPI_NVDIMM=y
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 781eafaf5e..f086adfa82 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -784,6 +784,7 @@ static
>>  void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>>  {
>>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>> +    MachineState *ms = MACHINE(vms);
>>      GArray *table_offsets;
>>      unsigned dsdt, xsdt;
>>      GArray *tables_blob = tables->table_data;
>> @@ -824,6 +825,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>>          }
>>      }
>>  
>> +    if (vms->acpi_nvdimm_state.is_enabled) {
>> +        nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
>> +                          &vms->acpi_nvdimm_state, ms->ram_slots);
>> +    }
>> +
>>      if (its_class_name() && !vmc->no_its) {
>>          acpi_add_table(table_offsets, tables_blob);
>>          build_iort(tables_blob, tables->linker, vms);
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index b683902991..0c8c2cc191 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -132,6 +132,7 @@ static const MemMapEntry a15memmap[] = {
>>      [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
>>      [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
>>      [VIRT_SMMU] =               { 0x09050000, 0x00020000 },
>> +    [VIRT_ACPI_IO] =            { 0x09070000, 0x00010000 },
> where does this range come from and is its size sufficient?
I understand it can be anywhere in low mem and must be large enough to
contain [NVDIMM_ACPI_IO_BASE, NDIMM_ACPI_IO_BASE + NVDIMM_ACPI_IO_LEN].
So one 64kB page should do the job?

Thanks

Eric
> 
>>      [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
>>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>>      [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
>> @@ -1637,6 +1638,18 @@ static void machvirt_init(MachineState *machine)
>>  
>>      create_platform_bus(vms, pic);
>>  
>> +    if (vms->acpi_nvdimm_state.is_enabled) {
>> +        AcpiNVDIMMState *acpi_nvdimm_state = &vms->acpi_nvdimm_state;
>> +
>> +        acpi_nvdimm_state->dsm_io.type = NVDIMM_ACPI_IO_MEMORY;
>> +        acpi_nvdimm_state->dsm_io.base =
>> +                vms->memmap[VIRT_ACPI_IO].base + NVDIMM_ACPI_IO_BASE;
>> +        acpi_nvdimm_state->dsm_io.len = NVDIMM_ACPI_IO_LEN;
>> +
>> +        nvdimm_init_acpi_state(acpi_nvdimm_state, sysmem,
>> +                               vms->fw_cfg, OBJECT(vms));
>> +    }
>> +
>>      vms->bootinfo.ram_size = machine->ram_size;
>>      vms->bootinfo.kernel_filename = machine->kernel_filename;
>>      vms->bootinfo.kernel_cmdline = machine->kernel_cmdline;
>> @@ -1822,10 +1835,19 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
>>                               DeviceState *dev, Error **errp)
>>  {
>>      VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
>> +    bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
>>      Error *local_err = NULL;
>>  
>>      pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err);
>> +    if (local_err) {
>> +        goto out;
>> +    }
>>  
>> +    if (is_nvdimm) {
>> +        nvdimm_plug(&vms->acpi_nvdimm_state);
>> +    }
>> +
>> +out:
>>      error_propagate(errp, local_err);
>>  }
>>  
>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>> index c88f67a492..56d73b0e86 100644
>> --- a/include/hw/arm/virt.h
>> +++ b/include/hw/arm/virt.h
>> @@ -37,6 +37,7 @@
>>  #include "hw/arm/arm.h"
>>  #include "sysemu/kvm.h"
>>  #include "hw/intc/arm_gicv3_common.h"
>> +#include "hw/mem/nvdimm.h"
>>  
>>  #define NUM_GICV2M_SPIS       64
>>  #define NUM_VIRTIO_TRANSPORTS 32
>> @@ -77,6 +78,7 @@ enum {
>>      VIRT_GPIO,
>>      VIRT_SECURE_UART,
>>      VIRT_SECURE_MEM,
>> +    VIRT_ACPI_IO,
>>      VIRT_LOWMEMMAP_LAST,
>>  };
>>  
>> @@ -134,6 +136,7 @@ typedef struct {
>>      hwaddr high_io_base;
>>      hwaddr highest_gpa;
>>      bool extended_memmap;
>> +    AcpiNVDIMMState acpi_nvdimm_state;
>>  } VirtMachineState;
>>  
>>  #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
>
Igor Mammedov Feb. 21, 2019, 12:16 p.m. UTC | #3
On Wed, 20 Feb 2019 16:21:05 +0100
Auger Eric <eric.auger@redhat.com> wrote:

> Hi Igor,
> 
> On 2/18/19 11:30 AM, Igor Mammedov wrote:
> > On Tue,  5 Feb 2019 18:33:04 +0100
> > Eric Auger <eric.auger@redhat.com> wrote:
> >   
> >> From: Kwangwoo Lee <kwangwoo.lee@sk.com>
> >>
> >> Pre-plug and plug handlers are prepared for NVDIMM support.
> >>
> >> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >> Signed-off-by: Kwangwoo Lee <kwangwoo.lee@sk.com>
> >> ---
> >>  default-configs/arm-softmmu.mak |  2 ++
> >>  hw/arm/virt-acpi-build.c        |  6 ++++++
> >>  hw/arm/virt.c                   | 22 ++++++++++++++++++++++
> >>  include/hw/arm/virt.h           |  3 +++
> >>  4 files changed, 33 insertions(+)
> >>
> >> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> >> index dc4624794f..ddbe87ed15 100644
> >> --- a/default-configs/arm-softmmu.mak
> >> +++ b/default-configs/arm-softmmu.mak
> >> @@ -162,3 +162,5 @@ CONFIG_HIGHBANK=y
> >>  CONFIG_MUSICPAL=y
> >>  CONFIG_MEM_DEVICE=y
> >>  CONFIG_DIMM=y
> >> +CONFIG_NVDIMM=y
> >> +CONFIG_ACPI_NVDIMM=y
> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >> index 781eafaf5e..f086adfa82 100644
> >> --- a/hw/arm/virt-acpi-build.c
> >> +++ b/hw/arm/virt-acpi-build.c
> >> @@ -784,6 +784,7 @@ static
> >>  void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
> >>  {
> >>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> >> +    MachineState *ms = MACHINE(vms);
> >>      GArray *table_offsets;
> >>      unsigned dsdt, xsdt;
> >>      GArray *tables_blob = tables->table_data;
> >> @@ -824,6 +825,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
> >>          }
> >>      }
> >>  
> >> +    if (vms->acpi_nvdimm_state.is_enabled) {
> >> +        nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
> >> +                          &vms->acpi_nvdimm_state, ms->ram_slots);
> >> +    }
> >> +
> >>      if (its_class_name() && !vmc->no_its) {
> >>          acpi_add_table(table_offsets, tables_blob);
> >>          build_iort(tables_blob, tables->linker, vms);
> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >> index b683902991..0c8c2cc191 100644
> >> --- a/hw/arm/virt.c
> >> +++ b/hw/arm/virt.c
> >> @@ -132,6 +132,7 @@ static const MemMapEntry a15memmap[] = {
> >>      [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
> >>      [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
> >>      [VIRT_SMMU] =               { 0x09050000, 0x00020000 },
> >> +    [VIRT_ACPI_IO] =            { 0x09070000, 0x00010000 },  
> > where does this range come from and is its size sufficient?  
> I understand it can be anywhere in low mem and must be large enough to
> contain [NVDIMM_ACPI_IO_BASE, NDIMM_ACPI_IO_BASE + NVDIMM_ACPI_IO_LEN].
it looked to like generic ACPI arrea rather than NVDIMM
so I'd suggest to name it properly and probably use NVDIMM_ACPI_IO_BASE & co
to add this entry so that reader won't have to wonder where this magic numbers
come from.

> So one 64kB page should do the job?
Should it be located in lowmem?
(do we care about device_memory & AVMF & ACPI & not 64bit guests?)


> Thanks
> 
> Eric
> >   
> >>      [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
> >>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> >>      [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
> >> @@ -1637,6 +1638,18 @@ static void machvirt_init(MachineState *machine)
> >>  
> >>      create_platform_bus(vms, pic);
> >>  
> >> +    if (vms->acpi_nvdimm_state.is_enabled) {
> >> +        AcpiNVDIMMState *acpi_nvdimm_state = &vms->acpi_nvdimm_state;
> >> +
> >> +        acpi_nvdimm_state->dsm_io.type = NVDIMM_ACPI_IO_MEMORY;
> >> +        acpi_nvdimm_state->dsm_io.base =
> >> +                vms->memmap[VIRT_ACPI_IO].base + NVDIMM_ACPI_IO_BASE;
> >> +        acpi_nvdimm_state->dsm_io.len = NVDIMM_ACPI_IO_LEN;
> >> +
> >> +        nvdimm_init_acpi_state(acpi_nvdimm_state, sysmem,
> >> +                               vms->fw_cfg, OBJECT(vms));
> >> +    }
> >> +
> >>      vms->bootinfo.ram_size = machine->ram_size;
> >>      vms->bootinfo.kernel_filename = machine->kernel_filename;
> >>      vms->bootinfo.kernel_cmdline = machine->kernel_cmdline;
> >> @@ -1822,10 +1835,19 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
> >>                               DeviceState *dev, Error **errp)
> >>  {
> >>      VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> >> +    bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
> >>      Error *local_err = NULL;
> >>  
> >>      pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err);
> >> +    if (local_err) {
> >> +        goto out;
> >> +    }
> >>  
> >> +    if (is_nvdimm) {
> >> +        nvdimm_plug(&vms->acpi_nvdimm_state);
> >> +    }
> >> +
> >> +out:
> >>      error_propagate(errp, local_err);
> >>  }
> >>  
> >> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> >> index c88f67a492..56d73b0e86 100644
> >> --- a/include/hw/arm/virt.h
> >> +++ b/include/hw/arm/virt.h
> >> @@ -37,6 +37,7 @@
> >>  #include "hw/arm/arm.h"
> >>  #include "sysemu/kvm.h"
> >>  #include "hw/intc/arm_gicv3_common.h"
> >> +#include "hw/mem/nvdimm.h"
> >>  
> >>  #define NUM_GICV2M_SPIS       64
> >>  #define NUM_VIRTIO_TRANSPORTS 32
> >> @@ -77,6 +78,7 @@ enum {
> >>      VIRT_GPIO,
> >>      VIRT_SECURE_UART,
> >>      VIRT_SECURE_MEM,
> >> +    VIRT_ACPI_IO,
> >>      VIRT_LOWMEMMAP_LAST,
> >>  };
> >>  
> >> @@ -134,6 +136,7 @@ typedef struct {
> >>      hwaddr high_io_base;
> >>      hwaddr highest_gpa;
> >>      bool extended_memmap;
> >> +    AcpiNVDIMMState acpi_nvdimm_state;
> >>  } VirtMachineState;
> >>  
> >>  #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)  
> >
Auger Eric Feb. 21, 2019, 12:34 p.m. UTC | #4
Hi Igor,

On 2/21/19 1:16 PM, Igor Mammedov wrote:
> On Wed, 20 Feb 2019 16:21:05 +0100
> Auger Eric <eric.auger@redhat.com> wrote:
> 
>> Hi Igor,
>>
>> On 2/18/19 11:30 AM, Igor Mammedov wrote:
>>> On Tue,  5 Feb 2019 18:33:04 +0100
>>> Eric Auger <eric.auger@redhat.com> wrote:
>>>   
>>>> From: Kwangwoo Lee <kwangwoo.lee@sk.com>
>>>>
>>>> Pre-plug and plug handlers are prepared for NVDIMM support.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> Signed-off-by: Kwangwoo Lee <kwangwoo.lee@sk.com>
>>>> ---
>>>>  default-configs/arm-softmmu.mak |  2 ++
>>>>  hw/arm/virt-acpi-build.c        |  6 ++++++
>>>>  hw/arm/virt.c                   | 22 ++++++++++++++++++++++
>>>>  include/hw/arm/virt.h           |  3 +++
>>>>  4 files changed, 33 insertions(+)
>>>>
>>>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>>>> index dc4624794f..ddbe87ed15 100644
>>>> --- a/default-configs/arm-softmmu.mak
>>>> +++ b/default-configs/arm-softmmu.mak
>>>> @@ -162,3 +162,5 @@ CONFIG_HIGHBANK=y
>>>>  CONFIG_MUSICPAL=y
>>>>  CONFIG_MEM_DEVICE=y
>>>>  CONFIG_DIMM=y
>>>> +CONFIG_NVDIMM=y
>>>> +CONFIG_ACPI_NVDIMM=y
>>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>>> index 781eafaf5e..f086adfa82 100644
>>>> --- a/hw/arm/virt-acpi-build.c
>>>> +++ b/hw/arm/virt-acpi-build.c
>>>> @@ -784,6 +784,7 @@ static
>>>>  void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>>>>  {
>>>>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>>>> +    MachineState *ms = MACHINE(vms);
>>>>      GArray *table_offsets;
>>>>      unsigned dsdt, xsdt;
>>>>      GArray *tables_blob = tables->table_data;
>>>> @@ -824,6 +825,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>>>>          }
>>>>      }
>>>>  
>>>> +    if (vms->acpi_nvdimm_state.is_enabled) {
>>>> +        nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
>>>> +                          &vms->acpi_nvdimm_state, ms->ram_slots);
>>>> +    }
>>>> +
>>>>      if (its_class_name() && !vmc->no_its) {
>>>>          acpi_add_table(table_offsets, tables_blob);
>>>>          build_iort(tables_blob, tables->linker, vms);
>>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>>> index b683902991..0c8c2cc191 100644
>>>> --- a/hw/arm/virt.c
>>>> +++ b/hw/arm/virt.c
>>>> @@ -132,6 +132,7 @@ static const MemMapEntry a15memmap[] = {
>>>>      [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
>>>>      [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
>>>>      [VIRT_SMMU] =               { 0x09050000, 0x00020000 },
>>>> +    [VIRT_ACPI_IO] =            { 0x09070000, 0x00010000 },  
>>> where does this range come from and is its size sufficient?  
>> I understand it can be anywhere in low mem and must be large enough to
>> contain [NVDIMM_ACPI_IO_BASE, NDIMM_ACPI_IO_BASE + NVDIMM_ACPI_IO_LEN].
> it looked to like generic ACPI arrea rather than NVDIMM
> so I'd suggest to name it properly and probably use NVDIMM_ACPI_IO_BASE & co
> to add this entry so that reader won't have to wonder where this magic numbers
> come from.
OK. I will rename the region in v8
> 
>> So one 64kB page should do the job?
> Should it be located in lowmem?
> (do we care about device_memory & AVMF & ACPI & not 64bit guests?)
I guess no. We could easily fit this region into the existing low mem
IO, that's what I meant actually.

Thanks

Eric
> 
> 
>> Thanks
>>
>> Eric
>>>   
>>>>      [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
>>>>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>>>>      [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
>>>> @@ -1637,6 +1638,18 @@ static void machvirt_init(MachineState *machine)
>>>>  
>>>>      create_platform_bus(vms, pic);
>>>>  
>>>> +    if (vms->acpi_nvdimm_state.is_enabled) {
>>>> +        AcpiNVDIMMState *acpi_nvdimm_state = &vms->acpi_nvdimm_state;
>>>> +
>>>> +        acpi_nvdimm_state->dsm_io.type = NVDIMM_ACPI_IO_MEMORY;
>>>> +        acpi_nvdimm_state->dsm_io.base =
>>>> +                vms->memmap[VIRT_ACPI_IO].base + NVDIMM_ACPI_IO_BASE;
>>>> +        acpi_nvdimm_state->dsm_io.len = NVDIMM_ACPI_IO_LEN;
>>>> +
>>>> +        nvdimm_init_acpi_state(acpi_nvdimm_state, sysmem,
>>>> +                               vms->fw_cfg, OBJECT(vms));
>>>> +    }
>>>> +
>>>>      vms->bootinfo.ram_size = machine->ram_size;
>>>>      vms->bootinfo.kernel_filename = machine->kernel_filename;
>>>>      vms->bootinfo.kernel_cmdline = machine->kernel_cmdline;
>>>> @@ -1822,10 +1835,19 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
>>>>                               DeviceState *dev, Error **errp)
>>>>  {
>>>>      VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
>>>> +    bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
>>>>      Error *local_err = NULL;
>>>>  
>>>>      pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err);
>>>> +    if (local_err) {
>>>> +        goto out;
>>>> +    }
>>>>  
>>>> +    if (is_nvdimm) {
>>>> +        nvdimm_plug(&vms->acpi_nvdimm_state);
>>>> +    }
>>>> +
>>>> +out:
>>>>      error_propagate(errp, local_err);
>>>>  }
>>>>  
>>>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>>>> index c88f67a492..56d73b0e86 100644
>>>> --- a/include/hw/arm/virt.h
>>>> +++ b/include/hw/arm/virt.h
>>>> @@ -37,6 +37,7 @@
>>>>  #include "hw/arm/arm.h"
>>>>  #include "sysemu/kvm.h"
>>>>  #include "hw/intc/arm_gicv3_common.h"
>>>> +#include "hw/mem/nvdimm.h"
>>>>  
>>>>  #define NUM_GICV2M_SPIS       64
>>>>  #define NUM_VIRTIO_TRANSPORTS 32
>>>> @@ -77,6 +78,7 @@ enum {
>>>>      VIRT_GPIO,
>>>>      VIRT_SECURE_UART,
>>>>      VIRT_SECURE_MEM,
>>>> +    VIRT_ACPI_IO,
>>>>      VIRT_LOWMEMMAP_LAST,
>>>>  };
>>>>  
>>>> @@ -134,6 +136,7 @@ typedef struct {
>>>>      hwaddr high_io_base;
>>>>      hwaddr highest_gpa;
>>>>      bool extended_memmap;
>>>> +    AcpiNVDIMMState acpi_nvdimm_state;
>>>>  } VirtMachineState;
>>>>  
>>>>  #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)  
>>>   
> 
>

Patch
diff mbox series

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index dc4624794f..ddbe87ed15 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -162,3 +162,5 @@  CONFIG_HIGHBANK=y
 CONFIG_MUSICPAL=y
 CONFIG_MEM_DEVICE=y
 CONFIG_DIMM=y
+CONFIG_NVDIMM=y
+CONFIG_ACPI_NVDIMM=y
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 781eafaf5e..f086adfa82 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -784,6 +784,7 @@  static
 void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
 {
     VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
+    MachineState *ms = MACHINE(vms);
     GArray *table_offsets;
     unsigned dsdt, xsdt;
     GArray *tables_blob = tables->table_data;
@@ -824,6 +825,11 @@  void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
         }
     }
 
+    if (vms->acpi_nvdimm_state.is_enabled) {
+        nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
+                          &vms->acpi_nvdimm_state, ms->ram_slots);
+    }
+
     if (its_class_name() && !vmc->no_its) {
         acpi_add_table(table_offsets, tables_blob);
         build_iort(tables_blob, tables->linker, vms);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b683902991..0c8c2cc191 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -132,6 +132,7 @@  static const MemMapEntry a15memmap[] = {
     [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
     [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
     [VIRT_SMMU] =               { 0x09050000, 0x00020000 },
+    [VIRT_ACPI_IO] =            { 0x09070000, 0x00010000 },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -1637,6 +1638,18 @@  static void machvirt_init(MachineState *machine)
 
     create_platform_bus(vms, pic);
 
+    if (vms->acpi_nvdimm_state.is_enabled) {
+        AcpiNVDIMMState *acpi_nvdimm_state = &vms->acpi_nvdimm_state;
+
+        acpi_nvdimm_state->dsm_io.type = NVDIMM_ACPI_IO_MEMORY;
+        acpi_nvdimm_state->dsm_io.base =
+                vms->memmap[VIRT_ACPI_IO].base + NVDIMM_ACPI_IO_BASE;
+        acpi_nvdimm_state->dsm_io.len = NVDIMM_ACPI_IO_LEN;
+
+        nvdimm_init_acpi_state(acpi_nvdimm_state, sysmem,
+                               vms->fw_cfg, OBJECT(vms));
+    }
+
     vms->bootinfo.ram_size = machine->ram_size;
     vms->bootinfo.kernel_filename = machine->kernel_filename;
     vms->bootinfo.kernel_cmdline = machine->kernel_cmdline;
@@ -1822,10 +1835,19 @@  static void virt_memory_plug(HotplugHandler *hotplug_dev,
                              DeviceState *dev, Error **errp)
 {
     VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+    bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
     Error *local_err = NULL;
 
     pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err);
+    if (local_err) {
+        goto out;
+    }
 
+    if (is_nvdimm) {
+        nvdimm_plug(&vms->acpi_nvdimm_state);
+    }
+
+out:
     error_propagate(errp, local_err);
 }
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index c88f67a492..56d73b0e86 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -37,6 +37,7 @@ 
 #include "hw/arm/arm.h"
 #include "sysemu/kvm.h"
 #include "hw/intc/arm_gicv3_common.h"
+#include "hw/mem/nvdimm.h"
 
 #define NUM_GICV2M_SPIS       64
 #define NUM_VIRTIO_TRANSPORTS 32
@@ -77,6 +78,7 @@  enum {
     VIRT_GPIO,
     VIRT_SECURE_UART,
     VIRT_SECURE_MEM,
+    VIRT_ACPI_IO,
     VIRT_LOWMEMMAP_LAST,
 };
 
@@ -134,6 +136,7 @@  typedef struct {
     hwaddr high_io_base;
     hwaddr highest_gpa;
     bool extended_memmap;
+    AcpiNVDIMMState acpi_nvdimm_state;
 } VirtMachineState;
 
 #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)