[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup
From: |
Wei Yang |
Subject: |
Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup |
Date: |
Wed, 27 Feb 2019 21:25:47 +0000 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Wed, Feb 27, 2019 at 06:27:49PM +0100, Igor Mammedov wrote:
>On Wed, 27 Feb 2019 13:59:20 +0000
>Wei Yang <address@hidden> wrote:
>
>> On Wed, Feb 27, 2019 at 02:12:42PM +0100, Igor Mammedov wrote:
>> >On Mon, 25 Feb 2019 12:47:14 +0000
>> >Wei Yang <address@hidden> wrote:
>> >
>> >> On Mon, Feb 25, 2019 at 09:05:37AM +0100, Igor Mammedov wrote:
>> >> >On Sat, 23 Feb 2019 00:02:49 +0000
>> >> >Wei Yang <address@hidden> wrote:
>> >> >
>> >> >> On Thu, Feb 21, 2019 at 03:50:04PM +0100, Igor Mammedov wrote:
>> >> >> >On Wed, 20 Feb 2019 08:51:21 +0800
>> >> >> >Wei Yang <address@hidden> wrote:
>> >> >> >
>> >> >> >> Three trivial cleanup for pc-dimm.
>> >> >> >>
>> >> >> >> Patch [1] remove the check on class->hotpluggable since pc-dimm is
>> >> >> >> always
>> >> >> >> hotpluggable.
>> >> >> >> Patch [2] remove nvdimm_realize
>> >> >> >> Patch [2] remove pcdimm realize-callback
>> >> >> >even though this series doesn't break anything, I disagree with it
>> >> >> >conceptually as it makes device less abstracted and make it more
>> >> >> >dependent on how existing machine code uses it.
>> >> >> >I'd drop whole series.
>> >> >> >
>> >> >>
>> >> >> Is Patch [1] also make device more dependent on existing
>> >> >> implementation?
>> >> >yes, it's implementation detail that PCDIMM&Co are hotpluggable now.
>> >> >
>> >> >> For example, when we look at the counterpart of acpi_memory_plug_cb():
>> >> >>
>> >> >> acpi_pcihp_device_plug_cb
>> >> >>
>> >> >> which handle the pci device hotplug. We don't check the hotpluggable
>> >> >> property for pci devices.
>> >> >>
>> >> >> To me, this is a general rule for PCDIMM, they are hotpluggable.
>> >> >yes, PCDIMMs are hotpluggable but apci device (piix4pm/ich9/...)
>> >> >handling hotplug
>> >> >should ignore any non-hotpluggable one. If you remove check and then
>> >> >later
>> >> >someone else would add non-hotpluggable memory device or make PC-DIMMs
>> >> >or its
>> >> >variant of non-hotpluggable one, acpi device handling will break.
>> >> >Hence I'd prefer to keep the check.
>> >> >
>> >>
>> >> Ok, if we'd support un-hotpluggable mem device, we could retain this
>> >> check. But this maybe not a proper place.
>> >>
>> >> Based on my understanding, at this point, every thing has been done
>> >> properly. If this check pass, we will send an acpi interrupt to notify
>> >> the guest. In case this is an un-hotpluggable device, every thing looks
>> >> good but no effect in guest. Because we skip the notification.
>> >>
>> >> Maybe we need to move the check in pre-plug?
>> >And what would happen then and afterwards?
>> >
>> >Point is to make one of the handlers in chain to ignore plug event
>> >(i.e. do not generate SCI event) while the rest of handlers complete
>> >successfully mapping device in address space and whatever else.
>> >
>>
>> This will have a well prepared device in system but guest is not
>> notified, right?
>yes, it's theoretically possible to move check up the call stack
>to machine level and not call secondary hotplug handler on non hotplugged
>device but that again depends on what secondary hotplug handler does.
>I'd rather keep logic independent here unless there is good reason not
>to do so.
>
>
>> My idea to move the check in pre-plug will result the qemu return when
>> it see no SCI event will be generated, so there is no device created.
>>
>> I guess current behavior is a designed decision?
>I'd say so.
>PS:
>QEMU's hotplug_hadler API is misnamed at this point as it handles both
>cold (basic device wiring) and hotplug (processing hotplug).
>Maybe we should rename it but I'm not irritated enough by it to do so.
>
Got it, thanks.
>> >> >> For Patch[2][3], I agree with you.
>> >> >>
>> >>
>>
--
Wei Yang
Help you, Help me
- Re: [Qemu-devel] [PATCH v2 3/3] pc-dimm: revert "introduce realize callback", (continued)
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Xiao Guangrong, 2019/02/21
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Igor Mammedov, 2019/02/21
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Wei Yang, 2019/02/22
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Igor Mammedov, 2019/02/25
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Wei Yang, 2019/02/25
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Igor Mammedov, 2019/02/27
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Wei Yang, 2019/02/27
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Igor Mammedov, 2019/02/27
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup,
Wei Yang <=
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Wei Yang, 2019/02/27
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Igor Mammedov, 2019/02/28
- Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Wei Yang, 2019/02/28
Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup, Michael S. Tsirkin, 2019/02/21