Re: [Qemu-devel] [PATCH v2 0/3] PCDIMM cleanup

[Wei Yang]

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?

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?

>> >> For Patch[2][3], I agree with you.
>> >>   
>> 

-- 
Wei Yang
Help you, Help me