On Wed, Feb 27, 2019 at 06:27:49PM +0100, Igor Mammedov wrote: >On Wed, 27 Feb 2019 13:59:20 +0000 >Wei Yang <richard.weiy...@gmail.com> 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 <richard.weiy...@gmail.com> 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 <richard.weiy...@gmail.com> 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 <richardw.y...@linux.intel.com> 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
