[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] i386, acpi: check acpi_memory_hotplug capaci
From: |
Wei Yang |
Subject: |
Re: [Qemu-devel] [PATCH v2] i386, acpi: check acpi_memory_hotplug capacity in pre_plug |
Date: |
Tue, 19 Feb 2019 08:50:42 +0800 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
On Mon, Feb 18, 2019 at 02:53:36PM +0100, Igor Mammedov wrote:
>On Mon, 18 Feb 2019 13:21:29 +0000
>Wei Yang <address@hidden> wrote:
>
>> On Mon, Feb 18, 2019 at 01:56:02PM +0100, Igor Mammedov wrote:
>> >On Mon, 18 Feb 2019 12:13:24 +0000
>> >Wei Yang <address@hidden> wrote:
>> >
>> >> On Mon, Feb 18, 2019 at 10:50:34AM +0100, Igor Mammedov wrote:
>> >> >On Mon, 18 Feb 2019 09:13:33 +0800
>> >> >Wei Yang <address@hidden> wrote:
>> >> >
>> >> >> Currently we do device realization like below:
>> >> >>
>> >> >> hotplug_handler_pre_plug()
>> >> >> dc->realize()
>> >> >> hotplug_handler_plug()
>> >> >>
>> >> >> Before we do device realization and plug, we should allocate necessary
>> >> >> resources and check if memory-hotplug-support property is enabled.
>> >> >>
>> >> >> At the piix4 and ich9, the acpi_memory_hotplug property is checked in
>> >> > ^^^^^^^ is field name and not a property so
>> >> >s/acpi_memory_hotplug/memory-hotplug-support/
>> >> >s/in/at/
>> >> >
>> >> >> plug stage. This means that device has been realized and mapped into
>> >> >> guest address space 'pc_dimm_plug()' by the time acpi plug handler is
>> >> >> called, where it might fail and crash QEMU due to reaching
>> >> >> g_assert_not_reached() (piix4) or error_abort (ich9).
>> >> >
>> >> >
>> >> >> This patch abstract the check on acpi_memory_hotplug capacity in
>> >> >> pre_plug stage.
>> >> >maybe better would be:
>> >> >"Fix it by checking if memory hotplug is enabled at pre_plug stage
>> >> >where we can gracefully abort hotplug request."
>> >> >
>> >> >> [changelog rephrase from address@hidden
>> >> >this provides zero information for a commit reader,
>> >> >it should go under --- to chagelog
>> >> >
>> >>
>> >> Ok, maybe different community has different convention.
>> >>
>> >> Linux kernel mm subsystem maintainer suggest me to add above line.
>> >>
>> >> >> Signed-off-by: Wei Yang <address@hidden>
>> >> >> CC: Igor Mammedov <address@hidden>
>> >> >> CC: Eric Blake <address@hidden>
>> >> >>
>> >> >> ---
>> >> >> v2:
>> >> >> * rephrase change log
>> >> >one usually adds commenter's name here
>> >> > * like this (address@hidden)
>> >> >or
>> >> > * (address@hidden)
>> >> > - entry 1
>> >> > - entry 2
>> >> > * (address@hidden)
>> >> > - ...
>> >> >
>> >>
>> >> Well, I would change to this style.
>> >>
>> >> >> * apply this change to ich9
>> >> >> * use hotplug_handler_pre_plug() instead of open-coding check
>> >> >> ---
>> >> >> hw/acpi/ich9.c | 14 ++++++++++++--
>> >> >> hw/acpi/piix4.c | 14 ++++++++++++--
>> >> >> hw/i386/pc.c | 5 +++++
>> >> >> hw/isa/lpc_ich9.c | 1 +
>> >> >> include/hw/acpi/ich9.h | 2 ++
>> >> >> 5 files changed, 32 insertions(+), 4 deletions(-)
>> >> >>
>> >> >> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
>> >> >> index c5d8646abc..906a10f09a 100644
>> >> >> --- a/hw/acpi/ich9.c
>> >> >> +++ b/hw/acpi/ich9.c
>> >> >> @@ -483,13 +483,23 @@ void ich9_pm_add_properties(Object *obj,
>> >> >> ICH9LPCPMRegs *pm, Error **errp)
>> >> >> NULL);
>> >> >> }
>> >> >>
>> >> >> +void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> >> >> DeviceState *dev,
>> >> >> + Error **errp)
>> >> >broken alignment?
>> >> >
>> >> >run /scripts/checkpatch.pl on patches before submitting them
>> >>
>> >> I copied this from ich9_pm_device_plug_cb(), so thought this is the
>> >> correct style.
>> >>
>> >> Will adjust this according to checkpatch.pl.
>> >
>> >see CODING_STYLE
>> >
>>
>> Went throught this file, but not find the description of function
>> definition's second line.
>A patch fixing CODING_STYLE that is welcome
>
Hmm... writing document seems more difficult than writing code to me :-)
Let me have a try to see if my English is not that bad.
--
Wei Yang
Help you, Help me
- [Qemu-devel] [PATCH v2] i386, acpi: check acpi_memory_hotplug capacity in pre_plug, Wei Yang, 2019/02/17
- Re: [Qemu-devel] [PATCH v2] i386, acpi: check acpi_memory_hotplug capacity in pre_plug, Igor Mammedov, 2019/02/18
- Re: [Qemu-devel] [PATCH v2] i386, acpi: check acpi_memory_hotplug capacity in pre_plug, Wei Yang, 2019/02/18
- Re: [Qemu-devel] [PATCH v2] i386, acpi: check acpi_memory_hotplug capacity in pre_plug, Igor Mammedov, 2019/02/18
- Re: [Qemu-devel] [PATCH v2] i386, acpi: check acpi_memory_hotplug capacity in pre_plug, Wei Yang, 2019/02/18
- Re: [Qemu-devel] [PATCH v2] i386, acpi: check acpi_memory_hotplug capacity in pre_plug, Igor Mammedov, 2019/02/18
- Re: [Qemu-devel] [PATCH v2] i386, acpi: check acpi_memory_hotplug capacity in pre_plug,
Wei Yang <=