Re: [Qemu-devel] [RFC v3 07/15] hw/arm/virt: Add memory hotplug framework
On 03.07.2018 21:34, Auger Eric wrote:
> Hi David,
>
> On 07/03/2018 08:44 PM, David Hildenbrand wrote:
>> On 03.07.2018 09:19, Eric Auger wrote:
>>> From: Shameer Kolothum
>>>
>>> This patch adds the the memory hot-plug/hot-unplug infrastructure
>>> in machvirt.
>>>
>>> Signed-off-by: Eric Auger
>>> Signed-off-by: Shameer Kolothum
>>> Signed-off-by: Kwangwoo Lee
>>>
>>> ---
>>>
>>> v1 -> v2:
>>> - s/virt_dimm_plug|unplug/virt_memory_plug|unplug
>>> - s/pc_dimm_memory_plug/pc_dimm_plug
>>> - reworded title and commit message
>>> - added pre_plug cb
>>> - don't handle get_memory_region failure anymore
>>> ---
>>> default-configs/arm-softmmu.mak | 2 ++
>>> hw/arm/virt.c | 68
>>> -
>>> 2 files changed, 69 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/default-configs/arm-softmmu.mak
>>> b/default-configs/arm-softmmu.mak
>>> index 834d45c..28fe8f3 100644
>>> --- a/default-configs/arm-softmmu.mak
>>> +++ b/default-configs/arm-softmmu.mak
>>> @@ -152,3 +152,5 @@ CONFIG_PCI_DESIGNWARE=y
>>> CONFIG_STRONGARM=y
>>> CONFIG_HIGHBANK=y
>>> CONFIG_MUSICPAL=y
>>> +CONFIG_MEM_HOTPLUG=y
>>> +
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 6fefb78..7190962 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -60,6 +60,8 @@
>>> #include "standard-headers/linux/input.h"
>>> #include "hw/arm/smmuv3.h"
>>> #include "hw/acpi/acpi.h"
>>> +#include "hw/mem/pc-dimm.h"
>>> +#include "hw/mem/nvdimm.h"
>>>
>>> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>>> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
>>> @@ -1785,6 +1787,53 @@ static const CPUArchIdList
>>> *virt_possible_cpu_arch_ids(MachineState *ms)
>>> return ms->possible_cpus;
>>> }
>>>
>>> +static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState
>>> *dev,
>>> + Error **errp)
>>> +{
>>> +const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
>>> +
>>> +if (is_nvdimm) {
>>> +error_setg(errp, "nvdimm is not yet supported");
>>> +return;
>>> +}
>>
>> You mention that actual hotplug is not supported, only coldplug.
>> Wouldn't this be the right place to check for that? (only skimmed over
>> your patches, how do you handle that?)
> At the moment I don't check it. I did not look yet at ways to
> discriminate both cases.
Looking at dev->hotplugged should be enough I guess.
>
> Thanks
>
> Eric
>>
>>
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [RFC v3 07/15] hw/arm/virt: Add memory hotplug framework
Hi David,
On 07/03/2018 08:44 PM, David Hildenbrand wrote:
> On 03.07.2018 09:19, Eric Auger wrote:
>> From: Shameer Kolothum
>>
>> This patch adds the the memory hot-plug/hot-unplug infrastructure
>> in machvirt.
>>
>> Signed-off-by: Eric Auger
>> Signed-off-by: Shameer Kolothum
>> Signed-off-by: Kwangwoo Lee
>>
>> ---
>>
>> v1 -> v2:
>> - s/virt_dimm_plug|unplug/virt_memory_plug|unplug
>> - s/pc_dimm_memory_plug/pc_dimm_plug
>> - reworded title and commit message
>> - added pre_plug cb
>> - don't handle get_memory_region failure anymore
>> ---
>> default-configs/arm-softmmu.mak | 2 ++
>> hw/arm/virt.c | 68
>> -
>> 2 files changed, 69 insertions(+), 1 deletion(-)
>>
>> diff --git a/default-configs/arm-softmmu.mak
>> b/default-configs/arm-softmmu.mak
>> index 834d45c..28fe8f3 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -152,3 +152,5 @@ CONFIG_PCI_DESIGNWARE=y
>> CONFIG_STRONGARM=y
>> CONFIG_HIGHBANK=y
>> CONFIG_MUSICPAL=y
>> +CONFIG_MEM_HOTPLUG=y
>> +
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 6fefb78..7190962 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -60,6 +60,8 @@
>> #include "standard-headers/linux/input.h"
>> #include "hw/arm/smmuv3.h"
>> #include "hw/acpi/acpi.h"
>> +#include "hw/mem/pc-dimm.h"
>> +#include "hw/mem/nvdimm.h"
>>
>> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
>> @@ -1785,6 +1787,53 @@ static const CPUArchIdList
>> *virt_possible_cpu_arch_ids(MachineState *ms)
>> return ms->possible_cpus;
>> }
>>
>> +static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState
>> *dev,
>> + Error **errp)
>> +{
>> +const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
>> +
>> +if (is_nvdimm) {
>> +error_setg(errp, "nvdimm is not yet supported");
>> +return;
>> +}
>
> You mention that actual hotplug is not supported, only coldplug.
> Wouldn't this be the right place to check for that? (only skimmed over
> your patches, how do you handle that?)
At the moment I don't check it. I did not look yet at ways to
discriminate both cases.
Thanks
Eric
>
>
Re: [Qemu-devel] [RFC v3 07/15] hw/arm/virt: Add memory hotplug framework
Hi David,
On 07/03/2018 08:28 PM, David Hildenbrand wrote:
> On 03.07.2018 09:19, Eric Auger wrote:
>> From: Shameer Kolothum
>>
>> This patch adds the the memory hot-plug/hot-unplug infrastructure
>> in machvirt.
>>
>> Signed-off-by: Eric Auger
>> Signed-off-by: Shameer Kolothum
>> Signed-off-by: Kwangwoo Lee
>>
>> ---
>>
>> v1 -> v2:
>> - s/virt_dimm_plug|unplug/virt_memory_plug|unplug
>> - s/pc_dimm_memory_plug/pc_dimm_plug
>> - reworded title and commit message
>> - added pre_plug cb
>> - don't handle get_memory_region failure anymore
>> ---
>> default-configs/arm-softmmu.mak | 2 ++
>> hw/arm/virt.c | 68
>> -
>> 2 files changed, 69 insertions(+), 1 deletion(-)
>>
>> diff --git a/default-configs/arm-softmmu.mak
>> b/default-configs/arm-softmmu.mak
>> index 834d45c..28fe8f3 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -152,3 +152,5 @@ CONFIG_PCI_DESIGNWARE=y
>> CONFIG_STRONGARM=y
>> CONFIG_HIGHBANK=y
>> CONFIG_MUSICPAL=y
>> +CONFIG_MEM_HOTPLUG=y
>> +
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 6fefb78..7190962 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -60,6 +60,8 @@
>> #include "standard-headers/linux/input.h"
>> #include "hw/arm/smmuv3.h"
>> #include "hw/acpi/acpi.h"
>> +#include "hw/mem/pc-dimm.h"
>> +#include "hw/mem/nvdimm.h"
>>
>> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
>> @@ -1785,6 +1787,53 @@ static const CPUArchIdList
>> *virt_possible_cpu_arch_ids(MachineState *ms)
>> return ms->possible_cpus;
>> }
>>
>> +static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState
>> *dev,
>> + Error **errp)
>> +{
>> +const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
>> +
>> +if (is_nvdimm) {
>> +error_setg(errp, "nvdimm is not yet supported");
>> +return;
>> +}
>> +}
>> +
>> +static void virt_memory_plug(HotplugHandler *hotplug_dev,
>> + DeviceState *dev, Error **errp)
>> +{
>> +PCDIMMDevice *dimm = PC_DIMM(dev);
>> +PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>> +MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
>> +Error *local_err = NULL;
>> +uint64_t align;
>> +
>> +if (memory_region_get_alignment(mr)) {
>> +align = memory_region_get_alignment(mr);
>> +} else {
>> +/* by default we align on 64KB page size */
>> +align = SZ_64K;
>> +}
>
> After my latest re-factoring is applied
>
> 1. memory_region_get_alignment(mr) will never be 0
> 2. alignment detection will be handled internally
>
> So once you rebase to that version, just pass NULL for "*legacy_align"
Agreed. Thanks
Eric
>
>
Re: [Qemu-devel] [RFC v3 07/15] hw/arm/virt: Add memory hotplug framework
On 03.07.2018 09:19, Eric Auger wrote:
> From: Shameer Kolothum
>
> This patch adds the the memory hot-plug/hot-unplug infrastructure
> in machvirt.
>
> Signed-off-by: Eric Auger
> Signed-off-by: Shameer Kolothum
> Signed-off-by: Kwangwoo Lee
>
> ---
>
> v1 -> v2:
> - s/virt_dimm_plug|unplug/virt_memory_plug|unplug
> - s/pc_dimm_memory_plug/pc_dimm_plug
> - reworded title and commit message
> - added pre_plug cb
> - don't handle get_memory_region failure anymore
> ---
> default-configs/arm-softmmu.mak | 2 ++
> hw/arm/virt.c | 68
> -
> 2 files changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 834d45c..28fe8f3 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -152,3 +152,5 @@ CONFIG_PCI_DESIGNWARE=y
> CONFIG_STRONGARM=y
> CONFIG_HIGHBANK=y
> CONFIG_MUSICPAL=y
> +CONFIG_MEM_HOTPLUG=y
> +
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 6fefb78..7190962 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -60,6 +60,8 @@
> #include "standard-headers/linux/input.h"
> #include "hw/arm/smmuv3.h"
> #include "hw/acpi/acpi.h"
> +#include "hw/mem/pc-dimm.h"
> +#include "hw/mem/nvdimm.h"
>
> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -1785,6 +1787,53 @@ static const CPUArchIdList
> *virt_possible_cpu_arch_ids(MachineState *ms)
> return ms->possible_cpus;
> }
>
> +static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState
> *dev,
> + Error **errp)
> +{
> +const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
> +
> +if (is_nvdimm) {
> +error_setg(errp, "nvdimm is not yet supported");
> +return;
> +}
You mention that actual hotplug is not supported, only coldplug.
Wouldn't this be the right place to check for that? (only skimmed over
your patches, how do you handle that?)
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [RFC v3 07/15] hw/arm/virt: Add memory hotplug framework
On 03.07.2018 09:19, Eric Auger wrote:
> From: Shameer Kolothum
>
> This patch adds the the memory hot-plug/hot-unplug infrastructure
> in machvirt.
>
> Signed-off-by: Eric Auger
> Signed-off-by: Shameer Kolothum
> Signed-off-by: Kwangwoo Lee
>
> ---
>
> v1 -> v2:
> - s/virt_dimm_plug|unplug/virt_memory_plug|unplug
> - s/pc_dimm_memory_plug/pc_dimm_plug
> - reworded title and commit message
> - added pre_plug cb
> - don't handle get_memory_region failure anymore
> ---
> default-configs/arm-softmmu.mak | 2 ++
> hw/arm/virt.c | 68
> -
> 2 files changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 834d45c..28fe8f3 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -152,3 +152,5 @@ CONFIG_PCI_DESIGNWARE=y
> CONFIG_STRONGARM=y
> CONFIG_HIGHBANK=y
> CONFIG_MUSICPAL=y
> +CONFIG_MEM_HOTPLUG=y
> +
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 6fefb78..7190962 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -60,6 +60,8 @@
> #include "standard-headers/linux/input.h"
> #include "hw/arm/smmuv3.h"
> #include "hw/acpi/acpi.h"
> +#include "hw/mem/pc-dimm.h"
> +#include "hw/mem/nvdimm.h"
>
> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -1785,6 +1787,53 @@ static const CPUArchIdList
> *virt_possible_cpu_arch_ids(MachineState *ms)
> return ms->possible_cpus;
> }
>
> +static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState
> *dev,
> + Error **errp)
> +{
> +const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
> +
> +if (is_nvdimm) {
> +error_setg(errp, "nvdimm is not yet supported");
> +return;
> +}
> +}
> +
> +static void virt_memory_plug(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> +PCDIMMDevice *dimm = PC_DIMM(dev);
> +PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> +MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> +Error *local_err = NULL;
> +uint64_t align;
> +
> +if (memory_region_get_alignment(mr)) {
> +align = memory_region_get_alignment(mr);
> +} else {
> +/* by default we align on 64KB page size */
> +align = SZ_64K;
> +}
After my latest re-factoring is applied
1. memory_region_get_alignment(mr) will never be 0
2. alignment detection will be handled internally
So once you rebase to that version, just pass NULL for "*legacy_align"
--
Thanks,
David / dhildenb
[Qemu-devel] [RFC v3 07/15] hw/arm/virt: Add memory hotplug framework
From: Shameer Kolothum
This patch adds the the memory hot-plug/hot-unplug infrastructure
in machvirt.
Signed-off-by: Eric Auger
Signed-off-by: Shameer Kolothum
Signed-off-by: Kwangwoo Lee
---
v1 -> v2:
- s/virt_dimm_plug|unplug/virt_memory_plug|unplug
- s/pc_dimm_memory_plug/pc_dimm_plug
- reworded title and commit message
- added pre_plug cb
- don't handle get_memory_region failure anymore
---
default-configs/arm-softmmu.mak | 2 ++
hw/arm/virt.c | 68 -
2 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 834d45c..28fe8f3 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -152,3 +152,5 @@ CONFIG_PCI_DESIGNWARE=y
CONFIG_STRONGARM=y
CONFIG_HIGHBANK=y
CONFIG_MUSICPAL=y
+CONFIG_MEM_HOTPLUG=y
+
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 6fefb78..7190962 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -60,6 +60,8 @@
#include "standard-headers/linux/input.h"
#include "hw/arm/smmuv3.h"
#include "hw/acpi/acpi.h"
+#include "hw/mem/pc-dimm.h"
+#include "hw/mem/nvdimm.h"
#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -1785,6 +1787,53 @@ static const CPUArchIdList
*virt_possible_cpu_arch_ids(MachineState *ms)
return ms->possible_cpus;
}
+static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
+
+if (is_nvdimm) {
+error_setg(errp, "nvdimm is not yet supported");
+return;
+}
+}
+
+static void virt_memory_plug(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+PCDIMMDevice *dimm = PC_DIMM(dev);
+PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
+MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
+Error *local_err = NULL;
+uint64_t align;
+
+if (memory_region_get_alignment(mr)) {
+align = memory_region_get_alignment(mr);
+} else {
+/* by default we align on 64KB page size */
+align = SZ_64K;
+}
+
+pc_dimm_plug(dev, MACHINE(hotplug_dev), align, &local_err);
+
+error_propagate(errp, local_err);
+}
+
+static void virt_memory_unplug(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+pc_dimm_unplug(dev, MACHINE(hotplug_dev));
+object_unparent(OBJECT(dev));
+}
+
+static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
+DeviceState *dev, Error **errp)
+{
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+virt_memory_pre_plug(hotplug_dev, dev, errp);
+}
+}
+
static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
@@ -1796,12 +1845,27 @@ static void virt_machine_device_plug_cb(HotplugHandler
*hotplug_dev,
SYS_BUS_DEVICE(dev));
}
}
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+virt_memory_plug(hotplug_dev, dev, errp);
+}
+}
+
+static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+virt_memory_unplug(hotplug_dev, dev, errp);
+} else {
+error_setg(errp, "device unplug request for unsupported device"
+ " type: %s", object_get_typename(OBJECT(dev)));
+}
}
static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
DeviceState *dev)
{
-if (object_dynamic_cast(OBJECT(dev), TYPE_SYS_BUS_DEVICE)) {
+if (object_dynamic_cast(OBJECT(dev), TYPE_SYS_BUS_DEVICE) ||
+ (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM))) {
return HOTPLUG_HANDLER(machine);
}
@@ -1861,7 +1925,9 @@ static void virt_machine_class_init(ObjectClass *oc, void
*data)
mc->kvm_type = virt_kvm_type;
assert(!mc->get_hotplug_handler);
mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
+hc->pre_plug = virt_machine_device_pre_plug_cb;
hc->plug = virt_machine_device_plug_cb;
+hc->unplug = virt_machine_device_unplug_cb;
}
static const TypeInfo virt_machine_info = {
--
2.5.5
