Re: [Qemu-devel] [PATCH v7 00/17] ARM virt: Initial RAM expansion and PCDIMM/NVDIMM support

[Auger Eric <eric.auger@redhat.com>]

Hi Igor,

[..]

>  
>> What still remains fuzzy for me is in case of cold plug the mmio hotplug
>> control region part only is read (despite the slot selection of course)
>> and returns 0 for addr/size and also flags meaning the slot is not
>> enabled.
> If you mean guest reads 0s than it looks broken, could you show
> trace log with mhp_* tracepoints enabled during a dimm hotplug.

Please find the traces + cmd line on x86


/qemu-system-x86_64 -M
q35,usb=off,dump-guest-core=off,kernel_irqchip=split,nvdimm -cpu
Haswell,-hle,-rtm -smp 4,sockets=4,cores=1,threads=1 -m
16G,maxmem=32G,slots=4 -display none --enable-kvm -serial
tcp:localhost:4444,server -trace
events=/home/augere/UPSTREAM/qemu2/nvdimm.txt -qmp
unix:/home/augere/TEST/QEMU/qmp-sock,server,nowait -rtc
base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=delay -realtime
mlock=off -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global
PIIX4_PM.disable_s4=1 -boot strict=on -machine kernel_irqchip=split
-object
memory-backend-file,id=mem3,share,mem-path=/home/augere/TEST/QEMU/nv-dimm-3,size=2G,align=128M
-device nvdimm,memdev=mem3,id=dimm3,label-size=2M -object
memory-backend-file,id=mem4,share,mem-path=/home/augere/TEST/QEMU/nv-dimm-4,size=2G,align=128M
-device nvdimm,memdev=mem4,id=dimm4,label-size=2M -device
virtio-blk-pci,bus=pcie.0,scsi=off,drive=drv0,id=virtio-disk0,bootindex=1,werror=stop,rerror=stop
-drive
file=/home/augere/VM/IMAGES/x86_64-vm1-f28.raw,format=raw,if=none,cache=writethrough,id=drv0
-device virtio-net-pci,bus=pcie.0,netdev=nic0,mac=6a:f5:10:b1:3d:d2
-netdev
tap,id=nic0,script=/home/augere/TEST/SCRIPTS/qemu-ifup,downscript=/home/augere/TEST/SCRIPTS/qemu-ifdown,vhost=on
-net none -d guest_errors

******************************************************************
ioctl(TUNSETIFF): Device or resource busy
qemu-system-x86_64: -serial tcp:localhost:4444,server: info: QEMU
waiting for connection on: disconnected:tcp:::1:4444,server
qemu-system-x86_64: warning: global PIIX4_PM.disable_s3=1 not used
qemu-system-x86_64: warning: global PIIX4_PM.disable_s4=1 not used
29556@1551449303.339464:mhp_acpi_write_slot set active slot: 0x0
29556@1551449303.339496:mhp_acpi_read_addr_hi slot[0x0] addr hi: 0x0
29556@1551449303.339505:mhp_acpi_read_addr_lo slot[0x0] addr lo: 0x0
29556@1551449303.339512:mhp_acpi_read_size_hi slot[0x0] size hi: 0x0
29556@1551449303.339520:mhp_acpi_read_size_lo slot[0x0] size lo: 0x0
29556@1551449303.339563:mhp_acpi_write_slot set active slot: 0x0
29556@1551449303.339574:mhp_acpi_read_flags slot[0x0] flags: 0x0
29556@1551449303.339621:mhp_acpi_write_slot set active slot: 0x1
29556@1551449303.339643:mhp_acpi_read_addr_hi slot[0x1] addr hi: 0x0
29556@1551449303.339651:mhp_acpi_read_addr_lo slot[0x1] addr lo: 0x0
29556@1551449303.339659:mhp_acpi_read_size_hi slot[0x1] size hi: 0x0
29556@1551449303.339667:mhp_acpi_read_size_lo slot[0x1] size lo: 0x0
29556@1551449303.339705:mhp_acpi_write_slot set active slot: 0x1
29556@1551449303.339713:mhp_acpi_read_flags slot[0x1] flags: 0x0
29556@1551449303.339757:mhp_acpi_write_slot set active slot: 0x2
29556@1551449303.339779:mhp_acpi_read_addr_hi slot[0x2] addr hi: 0x0
29556@1551449303.339787:mhp_acpi_read_addr_lo slot[0x2] addr lo: 0x0
29556@1551449303.339796:mhp_acpi_read_size_hi slot[0x2] size hi: 0x0
29556@1551449303.339804:mhp_acpi_read_size_lo slot[0x2] size lo: 0x0
29556@1551449303.339861:mhp_acpi_write_slot set active slot: 0x2
29556@1551449303.339870:mhp_acpi_read_flags slot[0x2] flags: 0x0
29556@1551449303.339916:mhp_acpi_write_slot set active slot: 0x3
29556@1551449303.339944:mhp_acpi_read_addr_hi slot[0x3] addr hi: 0x0
29556@1551449303.339954:mhp_acpi_read_addr_lo slot[0x3] addr lo: 0x0
29556@1551449303.339963:mhp_acpi_read_size_hi slot[0x3] size hi: 0x0
29556@1551449303.339971:mhp_acpi_read_size_lo slot[0x3] size lo: 0x0
29556@1551449303.340012:mhp_acpi_write_slot set active slot: 0x3
29556@1551449303.340020:mhp_acpi_read_flags slot[0x3] flags: 0x0
29556@1551449303.439695:mhp_acpi_write_slot set active slot: 0x0
29556@1551449303.439713:mhp_acpi_read_flags slot[0x0] flags: 0x0
29556@1551449303.439733:mhp_acpi_write_slot set active slot: 0x1
29556@1551449303.439740:mhp_acpi_read_flags slot[0x1] flags: 0x0
29556@1551449303.439759:mhp_acpi_write_slot set active slot: 0x2
29556@1551449303.439767:mhp_acpi_read_flags slot[0x2] flags: 0x0
29556@1551449303.439793:mhp_acpi_write_slot set active slot: 0x3
29556@1551449303.439801:mhp_acpi_read_flags slot[0x3] flags: 0x0
29556@1551449303.539590:mhp_acpi_write_slot set active slot: 0x0
29556@1551449303.539606:mhp_acpi_read_flags slot[0x0] flags: 0x0
29556@1551449303.539627:mhp_acpi_write_slot set active slot: 0x1
29556@1551449303.539634:mhp_acpi_read_flags slot[0x1] flags: 0x0
29556@1551449303.539652:mhp_acpi_write_slot set active slot: 0x2
29556@1551449303.539659:mhp_acpi_read_flags slot[0x2] flags: 0x0
29556@1551449303.539677:mhp_acpi_write_slot set active slot: 0x3
29556@1551449303.539684:mhp_acpi_read_flags slot[0x3] flags: 0x0

That's the only traces I get until I get the login prompt.

Thanks

Eric


> 
>> So despite the slots are advertised as hotpluggable/enabled in
>> the SRAT; I am not sure for the OS it actually makes any difference
>> whether the DSDT definition blocks are described or not.
> SRAT isn't used fro informing guests about amount of present RAM,
> it holds affinity information for present and possible RAM
> 
>> To be honest I am afraid this is too late to add those additional
>> features for 4.0 now. This is going to jeopardize the first preliminary
>> part which is the introduction of the new memory map, allowing the
>> expansion of the initial RAM and paving the way for device memory
>> introduction. So I think I am going to resend the first 10 patches in a
>> standalone series. And we can iterate on the PCDIMM/NVDIMM parts
>> independently.
> sounds good to me, I'll try to review 1-10 today 
>  
>> Thanks
>>
>> Eric
>>>
>>>   
>>>>
>>>> Thanks,
>>>> Shameer
>>>>  
>>>>> Then would remain the GED/GPIO actual integration.
>>>>>
>>>>> Thanks
>>>>>
>>>>> Eric    
>>>>>>    
>>>>>>> Also don't DIMM slots already make sense in DT mode. Usually we accept
>>>>>>> to add one feature in DT and then in ACPI. For instance we can benefit    
>>>>>> usually it doesn't conflict with each other (at least I'm not aware of it)
>>>>>> but I see a problem with in this case.
>>>>>>    
>>>>>>> from nvdimm in dt mode right? So, considering an incremental approach I
>>>>>>> would be in favour of keeping the DT nodes.    
>>>>>> I'd guess it is the same as for DIMMs, ACPI support for NVDIMMs is much
>>>>>> more versatile.
>>>>>>
>>>>>> I consider target application of arm/virt as a board that's used to
>>>>>> run in production generic ACPI capable guest in most use cases and
>>>>>> various DT only guests as secondary ones. It's hard to make
>>>>>> both usecases be happy with defaults (that's probably  one of the
>>>>>> reasons why 'sbsa' board is being added).
>>>>>>
>>>>>> So I'd give priority to ACPI based arm/virt versus DT when defaults are
>>>>>> considered.
>>>>>>    
>>>>>>> Thanks
>>>>>>>
>>>>>>> Eric    
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>    
>>>>>>    
>>>
>>>   
>