Re: [Qemu-devel] [PATCH-for-4.2 v9 06/12] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot

2019-09-02 Thread Shameerali Kolothum Thodi
Hi Eric,

> -Original Message-
> From: Auger Eric [mailto:eric.au...@redhat.com]
> Sent: 01 September 2019 12:23
> To: Shameerali Kolothum Thodi ;
> qemu-devel@nongnu.org; qemu-...@nongnu.org; imamm...@redhat.com
> Cc: peter.mayd...@linaro.org; sa...@linux.intel.com;
> ard.biesheu...@linaro.org; Linuxarm ; xuwei (O)
> ; shannon.zha...@gmail.com;
> sebastien.bo...@intel.com; ler...@redhat.com
> Subject: Re: [Qemu-devel] [PATCH-for-4.2 v9 06/12] hw/arm/virt: Enable device
> memory cold/hot plug with ACPI boot
> 
> Hi Shameer,
> On 9/1/19 1:18 PM, Auger Eric wrote:
> > Hi Shameer,
> >
> > On 8/13/19 11:05 PM, Shameer Kolothum wrote:
> >> This initializes the GED device with base memory and irq, configures
> >> ged memory hotplug event and builds the corresponding aml code. With
> >> this, both hot and cold plug of device memory is enabled now for
> >> Guest with ACPI boot.
> >>
> >> Memory cold plug support with Guest DT boot is not yet supported.
> >
> > I think you should comment about bios-tables-test-allowed-diff.h update.
> > Can't you update the table instead of ignoring the test?
> >
> > Thanks
> >
> > Eric
> >>
> >> Signed-off-by: Shameer Kolothum
> >> 
> >> ---
> >> v8 --> v9
> >>  -Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
> >>  -Error propagation to _plug() handler.
> >>  -Removed R-by by Eric for now.
> >>
> >> v7 --> v8
> >>  -Changed no_acpi_dev to no_ged.
> >>  -Fixed 'dev' reference leak by object_new().
> >>  -Updated bios-tables-test-allowed-diff.h to avoid "make check"
> >>   failure.
> >>
> >> ---
> >>  hw/arm/Kconfig|  2 +
> >>  hw/arm/virt-acpi-build.c  | 16 +++
> >>  hw/arm/virt.c | 62
> ---
> >>  include/hw/arm/virt.h |  4 ++
> >>  tests/bios-tables-test-allowed-diff.h |  1 +
> >>  5 files changed, 78 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index
> >> 84961c17ab..ad7f7c089b 100644
> >> --- a/hw/arm/Kconfig
> >> +++ b/hw/arm/Kconfig
> >> @@ -22,6 +22,8 @@ config ARM_VIRT
> >>  select ACPI_PCI
> >>  select MEM_DEVICE
> >>  select DIMM
> >> +select ACPI_MEMORY_HOTPLUG
> >> +select ACPI_HW_REDUCED
> >>
> >>  config CHEETAH
> >>  bool
> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >> index 0afb372769..63fa845076 100644
> >> --- a/hw/arm/virt-acpi-build.c
> >> +++ b/hw/arm/virt-acpi-build.c
> >> @@ -40,6 +40,8 @@
> >>  #include "hw/acpi/aml-build.h"
> >>  #include "hw/acpi/utils.h"
> >>  #include "hw/acpi/pci.h"
> >> +#include "hw/acpi/memory_hotplug.h"
> >> +#include "hw/acpi/generic_event_device.h"
> >>  #include "hw/pci/pcie_host.h"
> >>  #include "hw/pci/pci.h"
> >>  #include "hw/arm/virt.h"
> >> @@ -705,6 +707,7 @@ static void
> >>  build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState
> >> *vms)  {
> >>  Aml *scope, *dsdt;
> >> +MachineState *ms = MACHINE(vms);
> >>  const MemMapEntry *memmap = vms->memmap;
> >>  const int *irqmap = vms->irqmap;
> >>
> >> @@ -729,6 +732,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> >>vms->highmem, vms->highmem_ecam);
> >>  acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> >> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> >> +if (vms->acpi_dev) {
> >> +build_ged_aml(scope, "\\_SB."GED_DEVICE,
> >> +  HOTPLUG_HANDLER(vms->acpi_dev),
> >> +  irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE,
> AML_SYSTEM_MEMORY,
> >> +  memmap[VIRT_ACPI_GED].base);
> >> +}
> >> +
> >> +if (vms->acpi_dev && ms->ram_slots) {
> >> +build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB",
> NULL,
> >> + AML_SYSTEM_MEMORY,
> >> +
> memmap[VIRT_PCDIMM_ACPI].base);
> >> +}
> >> +
> >>  acpi_dsdt_add_power_button(scope);
> >>
>

Re: [Qemu-devel] [PATCH-for-4.2 v9 06/12] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot

2019-09-02 Thread Auger Eric
Hi Shameer,

On 9/2/19 11:21 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
> 
>> -Original Message-
>> From: Auger Eric [mailto:eric.au...@redhat.com]
>> Sent: 01 September 2019 12:19
>> To: Shameerali Kolothum Thodi ;
>> qemu-devel@nongnu.org; qemu-...@nongnu.org; imamm...@redhat.com
>> Cc: peter.mayd...@linaro.org; shannon.zha...@gmail.com;
>> sa...@linux.intel.com; sebastien.bo...@intel.com; xuwei (O)
>> ; ler...@redhat.com; ard.biesheu...@linaro.org;
>> Linuxarm 
>> Subject: Re: [PATCH-for-4.2 v9 06/12] hw/arm/virt: Enable device memory
>> cold/hot plug with ACPI boot
>>
>> Hi Shameer,
>>
>> On 8/13/19 11:05 PM, Shameer Kolothum wrote:
>>> This initializes the GED device with base memory and irq, configures
>>> ged memory hotplug event and builds the corresponding aml code. With
>>> this, both hot and cold plug of device memory is enabled now for Guest
>>> with ACPI boot.
>>>
>>> Memory cold plug support with Guest DT boot is not yet supported.
>>
>> I think you should comment about bios-tables-test-allowed-diff.h update.
> 
> Ok. I will add that.
> 
>> Can't you update the table instead of ignoring the test?
> 
> I think that is not how it is handled now. The process is,
> 
> "Expected table change is then handled like this:
> 1. add table to diff allowed list
> 2. change generating code (can be combined with 1)
> 3. maintainer runs a script to update expected +
>blows away allowed diff list "
> https://patchwork.kernel.org/patch/10967339/

Ok. Thanks for the link.

Best Regards

Eric
> 
> Thanks,
> Shameer
> 
>>
>> Thanks
>>
>> Eric
>>>
>>> Signed-off-by: Shameer Kolothum 
>>> ---
>>> v8 --> v9
>>>  -Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
>>>  -Error propagation to _plug() handler.
>>>  -Removed R-by by Eric for now.
>>>
>>> v7 --> v8
>>>  -Changed no_acpi_dev to no_ged.
>>>  -Fixed 'dev' reference leak by object_new().
>>>  -Updated bios-tables-test-allowed-diff.h to avoid "make check"
>>>   failure.
>>>
>>> ---
>>>  hw/arm/Kconfig|  2 +
>>>  hw/arm/virt-acpi-build.c  | 16 +++
>>>  hw/arm/virt.c | 62
>> ---
>>>  include/hw/arm/virt.h |  4 ++
>>>  tests/bios-tables-test-allowed-diff.h |  1 +
>>>  5 files changed, 78 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index
>>> 84961c17ab..ad7f7c089b 100644
>>> --- a/hw/arm/Kconfig
>>> +++ b/hw/arm/Kconfig
>>> @@ -22,6 +22,8 @@ config ARM_VIRT
>>>  select ACPI_PCI
>>>  select MEM_DEVICE
>>>  select DIMM
>>> +select ACPI_MEMORY_HOTPLUG
>>> +select ACPI_HW_REDUCED
>>>
>>>  config CHEETAH
>>>  bool
>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
>>> 0afb372769..63fa845076 100644
>>> --- a/hw/arm/virt-acpi-build.c
>>> +++ b/hw/arm/virt-acpi-build.c
>>> @@ -40,6 +40,8 @@
>>>  #include "hw/acpi/aml-build.h"
>>>  #include "hw/acpi/utils.h"
>>>  #include "hw/acpi/pci.h"
>>> +#include "hw/acpi/memory_hotplug.h"
>>> +#include "hw/acpi/generic_event_device.h"
>>>  #include "hw/pci/pcie_host.h"
>>>  #include "hw/pci/pci.h"
>>>  #include "hw/arm/virt.h"
>>> @@ -705,6 +707,7 @@ static void
>>>  build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState
>>> *vms)  {
>>>  Aml *scope, *dsdt;
>>> +MachineState *ms = MACHINE(vms);
>>>  const MemMapEntry *memmap = vms->memmap;
>>>  const int *irqmap = vms->irqmap;
>>>
>>> @@ -729,6 +732,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>> VirtMachineState *vms)
>>>vms->highmem, vms->highmem_ecam);
>>>  acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
>>> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
>>> +if (vms->acpi_dev) {
>>> +build_ged_aml(scope, "\\_SB."GED_DEVICE,
>>> +  HOTPLUG_HANDLER(vms->acpi_dev),
>>> +  irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE,
>> AML_SYSTEM_MEMORY,
>>> +  memmap[VIRT_ACPI_GED].base);
>>> +}
>>> +
>>> +if (vms->acpi_dev && ms->ram_slots) {
>>> +build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB",
>> NULL,
>>> + AML_SYSTEM_MEMORY,
>>> +
>> memmap[VIRT_PCDIMM_ACPI].base);
>>> +}
>>> +
>>>  acpi_dsdt_add_power_button(scope);
>>>
>>>  aml_append(dsdt, scope);
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c index
>>> ef65e721d2..0949a227a9 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -66,6 +66,7 @@
>>>  #include "target/arm/internals.h"
>>>  #include "hw/mem/pc-dimm.h"
>>>  #include "hw/mem/nvdimm.h"
>>> +#include "hw/acpi/generic_event_device.h"
>>>
>>>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>>>  static void virt_##major##_##minor##_class_init(ObjectClass *oc,
>>> \ @@ -136,6 +137,8 @@ static const MemMapEntry base_memmap[] = {
>>>  [VIRT_GPIO] =   { 0x0903, 0x1000 },
>>>  [VIRT_SECURE_UART] ={ 0x0904, 0x

Re: [Qemu-devel] [PATCH-for-4.2 v9 06/12] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot

2019-09-02 Thread Shameerali Kolothum Thodi
Hi Eric,

> -Original Message-
> From: Auger Eric [mailto:eric.au...@redhat.com]
> Sent: 01 September 2019 12:19
> To: Shameerali Kolothum Thodi ;
> qemu-devel@nongnu.org; qemu-...@nongnu.org; imamm...@redhat.com
> Cc: peter.mayd...@linaro.org; shannon.zha...@gmail.com;
> sa...@linux.intel.com; sebastien.bo...@intel.com; xuwei (O)
> ; ler...@redhat.com; ard.biesheu...@linaro.org;
> Linuxarm 
> Subject: Re: [PATCH-for-4.2 v9 06/12] hw/arm/virt: Enable device memory
> cold/hot plug with ACPI boot
> 
> Hi Shameer,
> 
> On 8/13/19 11:05 PM, Shameer Kolothum wrote:
> > This initializes the GED device with base memory and irq, configures
> > ged memory hotplug event and builds the corresponding aml code. With
> > this, both hot and cold plug of device memory is enabled now for Guest
> > with ACPI boot.
> >
> > Memory cold plug support with Guest DT boot is not yet supported.
> 
> I think you should comment about bios-tables-test-allowed-diff.h update.

Ok. I will add that.

> Can't you update the table instead of ignoring the test?

I think that is not how it is handled now. The process is,

"Expected table change is then handled like this:
1. add table to diff allowed list
2. change generating code (can be combined with 1)
3. maintainer runs a script to update expected +
   blows away allowed diff list "
https://patchwork.kernel.org/patch/10967339/

Thanks,
Shameer

> 
> Thanks
> 
> Eric
> >
> > Signed-off-by: Shameer Kolothum 
> > ---
> > v8 --> v9
> >  -Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
> >  -Error propagation to _plug() handler.
> >  -Removed R-by by Eric for now.
> >
> > v7 --> v8
> >  -Changed no_acpi_dev to no_ged.
> >  -Fixed 'dev' reference leak by object_new().
> >  -Updated bios-tables-test-allowed-diff.h to avoid "make check"
> >   failure.
> >
> > ---
> >  hw/arm/Kconfig|  2 +
> >  hw/arm/virt-acpi-build.c  | 16 +++
> >  hw/arm/virt.c | 62
> ---
> >  include/hw/arm/virt.h |  4 ++
> >  tests/bios-tables-test-allowed-diff.h |  1 +
> >  5 files changed, 78 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index
> > 84961c17ab..ad7f7c089b 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -22,6 +22,8 @@ config ARM_VIRT
> >  select ACPI_PCI
> >  select MEM_DEVICE
> >  select DIMM
> > +select ACPI_MEMORY_HOTPLUG
> > +select ACPI_HW_REDUCED
> >
> >  config CHEETAH
> >  bool
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
> > 0afb372769..63fa845076 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -40,6 +40,8 @@
> >  #include "hw/acpi/aml-build.h"
> >  #include "hw/acpi/utils.h"
> >  #include "hw/acpi/pci.h"
> > +#include "hw/acpi/memory_hotplug.h"
> > +#include "hw/acpi/generic_event_device.h"
> >  #include "hw/pci/pcie_host.h"
> >  #include "hw/pci/pci.h"
> >  #include "hw/arm/virt.h"
> > @@ -705,6 +707,7 @@ static void
> >  build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState
> > *vms)  {
> >  Aml *scope, *dsdt;
> > +MachineState *ms = MACHINE(vms);
> >  const MemMapEntry *memmap = vms->memmap;
> >  const int *irqmap = vms->irqmap;
> >
> > @@ -729,6 +732,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> >vms->highmem, vms->highmem_ecam);
> >  acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> > (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> > +if (vms->acpi_dev) {
> > +build_ged_aml(scope, "\\_SB."GED_DEVICE,
> > +  HOTPLUG_HANDLER(vms->acpi_dev),
> > +  irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE,
> AML_SYSTEM_MEMORY,
> > +  memmap[VIRT_ACPI_GED].base);
> > +}
> > +
> > +if (vms->acpi_dev && ms->ram_slots) {
> > +build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB",
> NULL,
> > + AML_SYSTEM_MEMORY,
> > +
> memmap[VIRT_PCDIMM_ACPI].base);
> > +}
> > +
> >  acpi_dsdt_add_power_button(scope);
> >
> >  aml_append(dsdt, scope);
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c index
> > ef65e721d2..0949a227a9 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -66,6 +66,7 @@
> >  #include "target/arm/internals.h"
> >  #include "hw/mem/pc-dimm.h"
> >  #include "hw/mem/nvdimm.h"
> > +#include "hw/acpi/generic_event_device.h"
> >
> >  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
> >  static void virt_##major##_##minor##_class_init(ObjectClass *oc,
> > \ @@ -136,6 +137,8 @@ static const MemMapEntry base_memmap[] = {
> >  [VIRT_GPIO] =   { 0x0903, 0x1000 },
> >  [VIRT_SECURE_UART] ={ 0x0904, 0x1000 },
> >  [VIRT_SMMU] =   { 0x0905, 0x0002 },
> > +[VIRT_PCDIMM_ACPI] ={ 0x0907,
> MEMORY_HOTPLUG_IO_LEN },
> > +[VIRT_

Re: [Qemu-devel] [PATCH-for-4.2 v9 06/12] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot

2019-09-01 Thread Auger Eric
Hi Shameer,
On 9/1/19 1:18 PM, Auger Eric wrote:
> Hi Shameer,
> 
> On 8/13/19 11:05 PM, Shameer Kolothum wrote:
>> This initializes the GED device with base memory and irq, configures
>> ged memory hotplug event and builds the corresponding aml code. With
>> this, both hot and cold plug of device memory is enabled now for Guest
>> with ACPI boot.
>>
>> Memory cold plug support with Guest DT boot is not yet supported.
> 
> I think you should comment about bios-tables-test-allowed-diff.h update.
> Can't you update the table instead of ignoring the test?
> 
> Thanks
> 
> Eric
>>
>> Signed-off-by: Shameer Kolothum 
>> ---
>> v8 --> v9
>>  -Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
>>  -Error propagation to _plug() handler.
>>  -Removed R-by by Eric for now.
>>
>> v7 --> v8
>>  -Changed no_acpi_dev to no_ged.
>>  -Fixed 'dev' reference leak by object_new().
>>  -Updated bios-tables-test-allowed-diff.h to avoid "make check"
>>   failure.
>>
>> ---
>>  hw/arm/Kconfig|  2 +
>>  hw/arm/virt-acpi-build.c  | 16 +++
>>  hw/arm/virt.c | 62 ---
>>  include/hw/arm/virt.h |  4 ++
>>  tests/bios-tables-test-allowed-diff.h |  1 +
>>  5 files changed, 78 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index 84961c17ab..ad7f7c089b 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -22,6 +22,8 @@ config ARM_VIRT
>>  select ACPI_PCI
>>  select MEM_DEVICE
>>  select DIMM
>> +select ACPI_MEMORY_HOTPLUG
>> +select ACPI_HW_REDUCED
>>  
>>  config CHEETAH
>>  bool
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 0afb372769..63fa845076 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -40,6 +40,8 @@
>>  #include "hw/acpi/aml-build.h"
>>  #include "hw/acpi/utils.h"
>>  #include "hw/acpi/pci.h"
>> +#include "hw/acpi/memory_hotplug.h"
>> +#include "hw/acpi/generic_event_device.h"
>>  #include "hw/pci/pcie_host.h"
>>  #include "hw/pci/pci.h"
>>  #include "hw/arm/virt.h"
>> @@ -705,6 +707,7 @@ static void
>>  build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>  {
>>  Aml *scope, *dsdt;
>> +MachineState *ms = MACHINE(vms);
>>  const MemMapEntry *memmap = vms->memmap;
>>  const int *irqmap = vms->irqmap;
>>  
>> @@ -729,6 +732,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 
>> VirtMachineState *vms)
>>vms->highmem, vms->highmem_ecam);
>>  acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
>> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
>> +if (vms->acpi_dev) {
>> +build_ged_aml(scope, "\\_SB."GED_DEVICE,
>> +  HOTPLUG_HANDLER(vms->acpi_dev),
>> +  irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, 
>> AML_SYSTEM_MEMORY,
>> +  memmap[VIRT_ACPI_GED].base);
>> +}
>> +
>> +if (vms->acpi_dev && ms->ram_slots) {
>> +build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
>> + AML_SYSTEM_MEMORY,
>> + memmap[VIRT_PCDIMM_ACPI].base);
>> +}
>> +
>>  acpi_dsdt_add_power_button(scope);
>>  
>>  aml_append(dsdt, scope);
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index ef65e721d2..0949a227a9 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -66,6 +66,7 @@
>>  #include "target/arm/internals.h"
>>  #include "hw/mem/pc-dimm.h"
>>  #include "hw/mem/nvdimm.h"
>> +#include "hw/acpi/generic_event_device.h"
>>  
>>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>>  static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
>> @@ -136,6 +137,8 @@ static const MemMapEntry base_memmap[] = {
>>  [VIRT_GPIO] =   { 0x0903, 0x1000 },
>>  [VIRT_SECURE_UART] ={ 0x0904, 0x1000 },
>>  [VIRT_SMMU] =   { 0x0905, 0x0002 },
>> +[VIRT_PCDIMM_ACPI] ={ 0x0907, MEMORY_HOTPLUG_IO_LEN },
>> +[VIRT_ACPI_GED] =   { 0x0908, ACPI_GED_EVT_SEL_LEN },
>>  [VIRT_MMIO] =   { 0x0a00, 0x0200 },
>>  /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size 
>> */
>>  [VIRT_PLATFORM_BUS] =   { 0x0c00, 0x0200 },
>> @@ -171,6 +174,7 @@ static const int a15irqmap[] = {
>>  [VIRT_PCIE] = 3, /* ... to 6 */
>>  [VIRT_GPIO] = 7,
>>  [VIRT_SECURE_UART] = 8,
>> +[VIRT_ACPI_GED] = 9,
>>  [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>>  [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>>  [VIRT_SMMU] = 74,/* ...to 74 + NUM_SMMU_IRQS - 1 */
>> @@ -520,6 +524,26 @@ static void fdt_add_pmu_nodes(const VirtMachineState 
>> *vms)
>>  }
>>  }
>>  
>> +static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq 
>> *pic)
>> +{
>> +DeviceState *dev;
>> +  

Re: [Qemu-devel] [PATCH-for-4.2 v9 06/12] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot

2019-09-01 Thread Auger Eric
Hi Shameer,

On 8/13/19 11:05 PM, Shameer Kolothum wrote:
> This initializes the GED device with base memory and irq, configures
> ged memory hotplug event and builds the corresponding aml code. With
> this, both hot and cold plug of device memory is enabled now for Guest
> with ACPI boot.
> 
> Memory cold plug support with Guest DT boot is not yet supported.

I think you should comment about bios-tables-test-allowed-diff.h update.
Can't you update the table instead of ignoring the test?

Thanks

Eric
> 
> Signed-off-by: Shameer Kolothum 
> ---
> v8 --> v9
>  -Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
>  -Error propagation to _plug() handler.
>  -Removed R-by by Eric for now.
> 
> v7 --> v8
>  -Changed no_acpi_dev to no_ged.
>  -Fixed 'dev' reference leak by object_new().
>  -Updated bios-tables-test-allowed-diff.h to avoid "make check"
>   failure.
> 
> ---
>  hw/arm/Kconfig|  2 +
>  hw/arm/virt-acpi-build.c  | 16 +++
>  hw/arm/virt.c | 62 ---
>  include/hw/arm/virt.h |  4 ++
>  tests/bios-tables-test-allowed-diff.h |  1 +
>  5 files changed, 78 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 84961c17ab..ad7f7c089b 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -22,6 +22,8 @@ config ARM_VIRT
>  select ACPI_PCI
>  select MEM_DEVICE
>  select DIMM
> +select ACPI_MEMORY_HOTPLUG
> +select ACPI_HW_REDUCED
>  
>  config CHEETAH
>  bool
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 0afb372769..63fa845076 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -40,6 +40,8 @@
>  #include "hw/acpi/aml-build.h"
>  #include "hw/acpi/utils.h"
>  #include "hw/acpi/pci.h"
> +#include "hw/acpi/memory_hotplug.h"
> +#include "hw/acpi/generic_event_device.h"
>  #include "hw/pci/pcie_host.h"
>  #include "hw/pci/pci.h"
>  #include "hw/arm/virt.h"
> @@ -705,6 +707,7 @@ static void
>  build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  {
>  Aml *scope, *dsdt;
> +MachineState *ms = MACHINE(vms);
>  const MemMapEntry *memmap = vms->memmap;
>  const int *irqmap = vms->irqmap;
>  
> @@ -729,6 +732,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>vms->highmem, vms->highmem_ecam);
>  acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> +if (vms->acpi_dev) {
> +build_ged_aml(scope, "\\_SB."GED_DEVICE,
> +  HOTPLUG_HANDLER(vms->acpi_dev),
> +  irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, 
> AML_SYSTEM_MEMORY,
> +  memmap[VIRT_ACPI_GED].base);
> +}
> +
> +if (vms->acpi_dev && ms->ram_slots) {
> +build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
> + AML_SYSTEM_MEMORY,
> + memmap[VIRT_PCDIMM_ACPI].base);
> +}
> +
>  acpi_dsdt_add_power_button(scope);
>  
>  aml_append(dsdt, scope);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ef65e721d2..0949a227a9 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -66,6 +66,7 @@
>  #include "target/arm/internals.h"
>  #include "hw/mem/pc-dimm.h"
>  #include "hw/mem/nvdimm.h"
> +#include "hw/acpi/generic_event_device.h"
>  
>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>  static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -136,6 +137,8 @@ static const MemMapEntry base_memmap[] = {
>  [VIRT_GPIO] =   { 0x0903, 0x1000 },
>  [VIRT_SECURE_UART] ={ 0x0904, 0x1000 },
>  [VIRT_SMMU] =   { 0x0905, 0x0002 },
> +[VIRT_PCDIMM_ACPI] ={ 0x0907, MEMORY_HOTPLUG_IO_LEN },
> +[VIRT_ACPI_GED] =   { 0x0908, ACPI_GED_EVT_SEL_LEN },
>  [VIRT_MMIO] =   { 0x0a00, 0x0200 },
>  /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size 
> */
>  [VIRT_PLATFORM_BUS] =   { 0x0c00, 0x0200 },
> @@ -171,6 +174,7 @@ static const int a15irqmap[] = {
>  [VIRT_PCIE] = 3, /* ... to 6 */
>  [VIRT_GPIO] = 7,
>  [VIRT_SECURE_UART] = 8,
> +[VIRT_ACPI_GED] = 9,
>  [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>  [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>  [VIRT_SMMU] = 74,/* ...to 74 + NUM_SMMU_IRQS - 1 */
> @@ -520,6 +524,26 @@ static void fdt_add_pmu_nodes(const VirtMachineState 
> *vms)
>  }
>  }
>  
> +static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq 
> *pic)
> +{
> +DeviceState *dev;
> +int irq = vms->irqmap[VIRT_ACPI_GED];
> +uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
> +
> +dev = qdev_create(NULL, TYPE_ACPI_GED);
> +qdev_prop_set_uint32(dev, "ged-event", e

[Qemu-devel] [PATCH-for-4.2 v9 06/12] hw/arm/virt: Enable device memory cold/hot plug with ACPI boot

2019-08-13 Thread Shameer Kolothum
This initializes the GED device with base memory and irq, configures
ged memory hotplug event and builds the corresponding aml code. With
this, both hot and cold plug of device memory is enabled now for Guest
with ACPI boot.

Memory cold plug support with Guest DT boot is not yet supported.

Signed-off-by: Shameer Kolothum 
---
v8 --> v9
 -Changes related to GED being a TYPE_SYS_BUS_DEVICE now.
 -Error propagation to _plug() handler.
 -Removed R-by by Eric for now.

v7 --> v8
 -Changed no_acpi_dev to no_ged.
 -Fixed 'dev' reference leak by object_new().
 -Updated bios-tables-test-allowed-diff.h to avoid "make check"
  failure.

---
 hw/arm/Kconfig|  2 +
 hw/arm/virt-acpi-build.c  | 16 +++
 hw/arm/virt.c | 62 ---
 include/hw/arm/virt.h |  4 ++
 tests/bios-tables-test-allowed-diff.h |  1 +
 5 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 84961c17ab..ad7f7c089b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -22,6 +22,8 @@ config ARM_VIRT
 select ACPI_PCI
 select MEM_DEVICE
 select DIMM
+select ACPI_MEMORY_HOTPLUG
+select ACPI_HW_REDUCED
 
 config CHEETAH
 bool
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 0afb372769..63fa845076 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -40,6 +40,8 @@
 #include "hw/acpi/aml-build.h"
 #include "hw/acpi/utils.h"
 #include "hw/acpi/pci.h"
+#include "hw/acpi/memory_hotplug.h"
+#include "hw/acpi/generic_event_device.h"
 #include "hw/pci/pcie_host.h"
 #include "hw/pci/pci.h"
 #include "hw/arm/virt.h"
@@ -705,6 +707,7 @@ static void
 build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
 Aml *scope, *dsdt;
+MachineState *ms = MACHINE(vms);
 const MemMapEntry *memmap = vms->memmap;
 const int *irqmap = vms->irqmap;
 
@@ -729,6 +732,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
   vms->highmem, vms->highmem_ecam);
 acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
(irqmap[VIRT_GPIO] + ARM_SPI_BASE));
+if (vms->acpi_dev) {
+build_ged_aml(scope, "\\_SB."GED_DEVICE,
+  HOTPLUG_HANDLER(vms->acpi_dev),
+  irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY,
+  memmap[VIRT_ACPI_GED].base);
+}
+
+if (vms->acpi_dev && ms->ram_slots) {
+build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
+ AML_SYSTEM_MEMORY,
+ memmap[VIRT_PCDIMM_ACPI].base);
+}
+
 acpi_dsdt_add_power_button(scope);
 
 aml_append(dsdt, scope);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ef65e721d2..0949a227a9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -66,6 +66,7 @@
 #include "target/arm/internals.h"
 #include "hw/mem/pc-dimm.h"
 #include "hw/mem/nvdimm.h"
+#include "hw/acpi/generic_event_device.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
 static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -136,6 +137,8 @@ static const MemMapEntry base_memmap[] = {
 [VIRT_GPIO] =   { 0x0903, 0x1000 },
 [VIRT_SECURE_UART] ={ 0x0904, 0x1000 },
 [VIRT_SMMU] =   { 0x0905, 0x0002 },
+[VIRT_PCDIMM_ACPI] ={ 0x0907, MEMORY_HOTPLUG_IO_LEN },
+[VIRT_ACPI_GED] =   { 0x0908, ACPI_GED_EVT_SEL_LEN },
 [VIRT_MMIO] =   { 0x0a00, 0x0200 },
 /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
 [VIRT_PLATFORM_BUS] =   { 0x0c00, 0x0200 },
@@ -171,6 +174,7 @@ static const int a15irqmap[] = {
 [VIRT_PCIE] = 3, /* ... to 6 */
 [VIRT_GPIO] = 7,
 [VIRT_SECURE_UART] = 8,
+[VIRT_ACPI_GED] = 9,
 [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
 [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
 [VIRT_SMMU] = 74,/* ...to 74 + NUM_SMMU_IRQS - 1 */
@@ -520,6 +524,26 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
 }
 }
 
+static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq 
*pic)
+{
+DeviceState *dev;
+int irq = vms->irqmap[VIRT_ACPI_GED];
+uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
+
+dev = qdev_create(NULL, TYPE_ACPI_GED);
+qdev_prop_set_uint32(dev, "ged-event", event);
+object_property_add_child(qdev_get_machine(), "acpi-ged",
+  OBJECT(dev), NULL);
+qdev_init_nofail(dev);
+
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_ACPI_GED].base);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, 
vms->memmap[VIRT_PCDIMM_ACPI].base);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
+
+return dev;
+}
+
 static void create_its(VirtMachineState *vms, DeviceState *gicd