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 >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>> >>> >>> >