Re: [Qemu-devel] [PATCH 00/16] implement vNVDIMM
On 07/03/2015 02:11 AM, Paolo Bonzini wrote: On 02/07/2015 20:01, Xiao Guangrong wrote: Thanks for your review, Stefan and Paolo! On 07/02/2015 05:52 PM, Paolo Bonzini wrote: On 02/07/2015 11:20, Stefan Hajnoczi wrote: Currently, the NVDIMM driver has been merged into upstream Linux Kernel and this patchset tries to enable it in virtualization field From a device model perspective, have you checked whether it makes sense to integrate nvdimms into the pc-dimm and hostmem code that is used for memory hotplug and NUMA? The NVDIMM device in your patches is a completely new TYPE_DEVICE so it doesn't share any interfaces or code with existing memory devices. Maybe that is the right solution here because NVDIMMs have different characteristics, but I'm not sure. The hostmem code should definitely be shared, e.g. by adding a new file property to the memory-backend-file class. ivshmem can also use it---CCing Marc-Andr�. However, file-based memory used by NVDIMM is special, it divides the file to two parts, one part is used as PMEM and another part is used to store NVDIMM's configure data. Maybe we can introduce end-reserved property to reserve specified size at the end of the file. Or create a new class type based on memory-backend-file (named nvdimm-backend-file) class to hide this magic thing? I need to read the code then. :) Paolo, do you have any comment? :) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 00/16] implement vNVDIMM
On Thu, Jul 02, 2015 at 09:31:23AM +0100, Stefan Hajnoczi wrote: On Thu, Jul 02, 2015 at 02:34:05PM +0800, Xiao Guangrong wrote: On 07/02/2015 02:17 PM, Michael S. Tsirkin wrote: On Wed, Jul 01, 2015 at 10:50:16PM +0800, Xiao Guangrong wrote: hw/acpi/aml-build.c | 32 +- hw/i386/acpi-build.c|9 +- hw/i386/acpi-dsdt.dsl |2 +- hw/i386/pc.c| 11 +- hw/mem/Makefile.objs|1 + hw/mem/pc-nvdimm.c | 1040 +++ include/hw/acpi/aml-build.h |5 +- include/hw/mem/pc-nvdimm.h | 56 +++ 8 files changed, 1149 insertions(+), 7 deletions(-) create mode 100644 hw/mem/pc-nvdimm.c create mode 100644 include/hw/mem/pc-nvdimm.h Given the amount of code, this is definitely not 2.4 material. Maybe others will have the time to review it before this, but in any case please remember to repost after 2.4 is out. I see, thanks for your reminder, Michael! I will review the series now. Here is the QEMU release schedule: http://qemu-project.org/Planning/2.4 Hard freeze - 7 July QEMU 2.4 release - 4 August It could be merged into a maintainer's tree when the -next branches are opened (it's up to each maintainer but for the block and net trees I do that at hard freeze time). Absolutely, but I'm not sure I'll do a next tree this time around. -- MST -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 00/16] implement vNVDIMM
On 02/07/2015 11:20, Stefan Hajnoczi wrote: Currently, the NVDIMM driver has been merged into upstream Linux Kernel and this patchset tries to enable it in virtualization field From a device model perspective, have you checked whether it makes sense to integrate nvdimms into the pc-dimm and hostmem code that is used for memory hotplug and NUMA? The NVDIMM device in your patches is a completely new TYPE_DEVICE so it doesn't share any interfaces or code with existing memory devices. Maybe that is the right solution here because NVDIMMs have different characteristics, but I'm not sure. The hostmem code should definitely be shared, e.g. by adding a new file property to the memory-backend-file class. ivshmem can also use it---CCing Marc-André. I don't know about the pc-dimm devices. If the NVDIMM devices can do _OST and can be hotplugged, then the answer is probably yes. Paolo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 00/16] implement vNVDIMM
On Thu, Jul 02, 2015 at 02:34:05PM +0800, Xiao Guangrong wrote: On 07/02/2015 02:17 PM, Michael S. Tsirkin wrote: On Wed, Jul 01, 2015 at 10:50:16PM +0800, Xiao Guangrong wrote: hw/acpi/aml-build.c | 32 +- hw/i386/acpi-build.c|9 +- hw/i386/acpi-dsdt.dsl |2 +- hw/i386/pc.c| 11 +- hw/mem/Makefile.objs|1 + hw/mem/pc-nvdimm.c | 1040 +++ include/hw/acpi/aml-build.h |5 +- include/hw/mem/pc-nvdimm.h | 56 +++ 8 files changed, 1149 insertions(+), 7 deletions(-) create mode 100644 hw/mem/pc-nvdimm.c create mode 100644 include/hw/mem/pc-nvdimm.h Given the amount of code, this is definitely not 2.4 material. Maybe others will have the time to review it before this, but in any case please remember to repost after 2.4 is out. I see, thanks for your reminder, Michael! I will review the series now. Here is the QEMU release schedule: http://qemu-project.org/Planning/2.4 Hard freeze - 7 July QEMU 2.4 release - 4 August It could be merged into a maintainer's tree when the -next branches are opened (it's up to each maintainer but for the block and net trees I do that at hard freeze time). pgpGg9qlhEWNe.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 00/16] implement vNVDIMM
On Wed, Jul 01, 2015 at 10:50:16PM +0800, Xiao Guangrong wrote: == Background == NVDIMM (A Non-Volatile Dual In-line Memory Module) is going to be supported on Intel's platform. They are discovered via ACPI and configured by _DSM method of NVDIMM device in ACPI. There has some supporting documents which can be found at: ACPI 6: http://www.uefi.org/sites/default/files/resources/ACPI_6.0.pdf NVDIMM Namespace: http://pmem.io/documents/NVDIMM_Namespace_Spec.pdf DSM Interface Example: http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf Driver Writer's Guide: http://pmem.io/documents/NVDIMM_Driver_Writers_Guide.pdf Currently, the NVDIMM driver has been merged into upstream Linux Kernel and this patchset tries to enable it in virtualization field From a device model perspective, have you checked whether it makes sense to integrate nvdimms into the pc-dimm and hostmem code that is used for memory hotplug and NUMA? The NVDIMM device in your patches is a completely new TYPE_DEVICE so it doesn't share any interfaces or code with existing memory devices. Maybe that is the right solution here because NVDIMMs have different characteristics, but I'm not sure. pgpbdYnHE2wZa.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 00/16] implement vNVDIMM
On 02/07/2015 20:01, Xiao Guangrong wrote: Thanks for your review, Stefan and Paolo! On 07/02/2015 05:52 PM, Paolo Bonzini wrote: On 02/07/2015 11:20, Stefan Hajnoczi wrote: Currently, the NVDIMM driver has been merged into upstream Linux Kernel and this patchset tries to enable it in virtualization field From a device model perspective, have you checked whether it makes sense to integrate nvdimms into the pc-dimm and hostmem code that is used for memory hotplug and NUMA? The NVDIMM device in your patches is a completely new TYPE_DEVICE so it doesn't share any interfaces or code with existing memory devices. Maybe that is the right solution here because NVDIMMs have different characteristics, but I'm not sure. The hostmem code should definitely be shared, e.g. by adding a new file property to the memory-backend-file class. ivshmem can also use it---CCing Marc-Andr�. However, file-based memory used by NVDIMM is special, it divides the file to two parts, one part is used as PMEM and another part is used to store NVDIMM's configure data. Maybe we can introduce end-reserved property to reserve specified size at the end of the file. Or create a new class type based on memory-backend-file (named nvdimm-backend-file) class to hide this magic thing? I need to read the code then. :) Paolo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 00/16] implement vNVDIMM
Thanks for your review, Stefan and Paolo! On 07/02/2015 05:52 PM, Paolo Bonzini wrote: On 02/07/2015 11:20, Stefan Hajnoczi wrote: Currently, the NVDIMM driver has been merged into upstream Linux Kernel and this patchset tries to enable it in virtualization field From a device model perspective, have you checked whether it makes sense to integrate nvdimms into the pc-dimm and hostmem code that is used for memory hotplug and NUMA? The NVDIMM device in your patches is a completely new TYPE_DEVICE so it doesn't share any interfaces or code with existing memory devices. Maybe that is the right solution here because NVDIMMs have different characteristics, but I'm not sure. The hostmem code should definitely be shared, e.g. by adding a new file property to the memory-backend-file class. ivshmem can also use it---CCing Marc-Andr�. However, file-based memory used by NVDIMM is special, it divides the file to two parts, one part is used as PMEM and another part is used to store NVDIMM's configure data. Maybe we can introduce end-reserved property to reserve specified size at the end of the file. Or create a new class type based on memory-backend-file (named nvdimm-backend-file) class to hide this magic thing? I don't know about the pc-dimm devices. If the NVDIMM devices can do _OST and can be hotplugged, then the answer is probably yes. _OST is not needed for NVDIMM. NVDIMM is completely different with dimm memory device in ACPI - it has different HID, method object, memory range detection, device organization, etc. So i prefer to introducing new device class for NVDIMM. For hotplug, NVDIMM and DIMM can share some logic, e.g, free-address-range management, slot management ... ( but new Object initiation in ACPI is complete different), we can abstract these operation as common part. NUMA detection is also different between NVDIMM, DIMM is also different, NVDIMM need to report its NUMA affinity in SPA table. But they can share some common function i think. BTW, i am going to implement vNVDIMM hotplug once linux NVDIMM driver supports it. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html